On Sun, 19 Aug 2018, Bernd Edlinger wrote: > Hi, > > > I rebased my range computation patch to current trunk, > and updated it according to what was discussed here. > > That means get_range_strlen has already a parameter > that is used to differentiate between ranges for warnings > and ranges for code-gen. > > That is called "strict", in the 4-parameter overload > and "fuzzy" in the internally used 7-parameter overload. > > So I added an "optimistic" parameter to my > get_inner_char_array_unless_typecast helper function. > That's it. > > Therefore at this time, there is only one warning regression > in one test case and one xfailed warning test case fixed. > > So that is par on the warning regression side. > > The failed test case is gcc/testsuite/gcc.dg/pr83373.c which > uses -fassume-zero-terminated-char-arrays, to enable the > (unsafe) feedback from string-length information to VRP to > suppress the warning. > > The 5 test cases that were designed to check the optimized > tree dump have to use the new -fassume-zero-terminated-char-arrays > option, but that is what we agreed upon. > > The patch is not dependent on any other patches. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk?
+ 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)) + || TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (base, 0)))) + != TYPE_MAIN_VARIANT (TREE_TYPE (base)))) I'm not convinced you are testing anything useful here. TREE_OPERAND (base, 1) might be a pointer which means it's type doesn't have any semantics so you are testing the access type against "random". If you'd restrict this to ADDR_EXPRs and look at the objects declared type then you'd still miss type-changes from a dynamic type that is different from what is declared. So my conclusion is if you really want to not want to return arg for things that look like a type cast then you have to unconditionally return NULL_TREE. + || TREE_CODE (base) == VIEW_CONVERT_EXPR + /* Or other stuff that would be handled by get_inner_reference. */ simply use || handled_component_p (base) for the above and the rest to be sure to handle everything that is not stripped above. + || TREE_CODE (base) == BIT_FIELD_REF + || TREE_CODE (base) == REALPART_EXPR + || TREE_CODE (base) == IMAGPART_EXPR) + return NULL_TREE; Btw, you are always returning the passed arg or NULL_TREE so formulating this as a predicate function makes uses easier. Not sure why it is called "inner" char array? There do seem to be independently useful fixes in the patch that I'd approve immediately. Btw, I don't think we want sth like flag_assume_zero_terminated_char_arrays or even make it default at -Ofast. Richard.