This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Make strlen range computations more conservative
- From: Richard Biener <rguenther at suse dot de>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jeff Law <law at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>
- Date: Tue, 24 Jul 2018 16:50:23 +0200 (CEST)
- Subject: Re: [PATCH] Make strlen range computations more conservative
- References: <AM5PR0701MB2657A771EAD27A906CE47D0AE4550@AM5PR0701MB2657.eurprd07.prod.outlook.com>
On Tue, 24 Jul 2018, Bernd Edlinger wrote:
> This patch makes strlen range computations more conservative.
> Firstly if there is a visible type cast from type A to B before passing
> then value to strlen, don't expect the type layout of B to restrict the
> possible return value range of strlen.
> Furthermore use the outermost enclosing array instead of the
> innermost one, because too aggressive optimization will likely
> convert harmless errors into security-relevant errors, because
> as the existing test cases demonstrate, this optimization is actively
> attacking string length checks in user code, while and not giving
> any warnings.
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
I'd like us to be explicit in what we support, not what we do not
+ /* Avoid arrays of pointers. */
+ if (TREE_CODE (TREE_TYPE (arg)) == POINTER_TYPE)
+ return false;
/* We handle arrays of integer types. */
if (TREE_CODE (TRE_TYPE (arg)) != INTEGER_TYPE)
+ tree base = arg;
+ while (TREE_CODE (base) == ARRAY_REF
+ || TREE_CODE (base) == ARRAY_RANGE_REF
+ || TREE_CODE (base) == COMPONENT_REF)
+ base = TREE_OPERAND (base, 0);
+ /* If this looks like a type cast don't assume anything. */
+ if ((TREE_CODE (base) == MEM_REF
+ && (! integer_zerop (TREE_OPERAND (base, 1))
+ || TREE_TYPE (TREE_TYPE (TREE_OPERAND (base, 0)))
+ != TREE_TYPE (base)))
+ || TREE_CODE (base) == VIEW_CONVERT_EXPR)
likewise - you miss to handle BIT_FIELD_REF. So, instead
if (!(DECL_P (base)
|| TREE_CODE (base) == STRING_CST
|| (TREE_CODE (base) == MEM_REF
you should look at comparing TYPE_MAIN_VARIANT in your type
check, aligned/unaligned or const/non-const accesses shouldn't
be considered a "type cast". Maybe even use
types_compatible_p. Not sure why you enforce zero-offset MEMs?
Do we in the end only handle &decl bases of MEMs? Given you
strip arbitrary COMPONENT_REFs the offset in a MEM isn't
so much different?
It looks like the component-ref stripping plus type-check part
could be factored out into sth like get_base_address? I don't
have a good name or suggested semantics for it though.
Richard Biener <email@example.com>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)