On 07/30/2018 03:11 PM, Bernd Edlinger wrote:
Hi,@@ -621,6 +674,12 @@ c_strlen (tree src, int only_value) maxelts = maxelts / eltsize - 1; } + /* Unless the caller is prepared to handle it by passing in a non-null + ARR, fail if the terminating nul doesn't fit in the array the string + is stored in (as in const char a[3] = "123"; */ + if (!arr && maxelts < strelts) + return NULL_TREE; +this is c_strlen, how is the caller ever supposed to handle non-zero terminated strings??? especially if you do this above?
Callers that pass in a non-null ARR handle them by issuing a warning. The rest get back a null result. It should be evident from the rest of the patch. It can be debated what each caller should do when it detects such a missing nul where one is expected. Different approaches may be more or less appropriate for different callers/functions (e.g., strcpy vs strlen).
+c_strlen (tree src, int only_value, tree *arr /* = NULL */) { STRIP_NOPS (src); + + /* Used to detect non-nul-terminated strings in subexpressions + of a conditional expression. When ARR is null, point it at + one of the elements for simplicity. */ + tree arrs[] = { NULL_TREE, NULL_TREE }; + if (!arr) + arr = arrs;@@ -11427,7 +11478,9 @@ string_constant (tree arg, tree *ptr_offset) unsigned HOST_WIDE_INT length = TREE_STRING_LENGTH (init); length = string_length (TREE_STRING_POINTER (init), charsize, length / charsize); - if (compare_tree_int (array_size, length + 1) < 0) + if (nulterm) + *nulterm = array_elts > length; + else if (array_elts <= length) return NULL_TREE;I don't understand why you can't use compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (init)), TREE_STRING_LENGTH (init)) instead of this convoluted code above ??? Sorry, this patch does not look like it is ready any time soon.
I'm open to technical comments on the substance of my changes but I'm not interested in your opinion of the readiness of the patch (whatever that might mean), certainly not if you have formed it after skimming a random handful of lines out of a 600 line patch.
But actually I am totally puzzled by your priorities. This is what I see right now: 1) We have missing warnings. 2) We have wrong code bugs. 3) We have apparently a specification error on the C Language standard (*) Why are you prioritizing 1) over 2) thus blocking my attempts to fix a wrong code issue,and why do you not tackle 3) in your WG14?
My priorities are none of your concern. Your "attempts to fix" issues interfere with my work on a number of projects. You are not being helpful -- instead, by submitting changes that you know fully well conflict with mine, you are impeding and undermining my work. That is why I object to them.
(*) which means that GCC is currently removing code from assertions as I pointed out here: https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01695.html This happens because GCC follows the language standards literally right now. I would say too literally, and it proves that the language standard's logic is flawed IMHO.
I have no idea what your point is about standards, but bugs like the one in the example, including those arising from uninitialized arrays, could be detected with only minor enhancements to the tree-ssa-strlen pass. Implementing some of this is among the projects I'm expected and expecting to work on for GCC 9. This patch is a small step in that direction. If you care about detecting bugs I would expect you to be supportive rather than dismissive of this work, and helpful in bringing it to fruition rather that putting it down or questioning my priorities. Especially since the work was prompted by your own (valid) complaint that GCC doesn't diagnose them. Martin
