On 20 May 2015 at 15:33, Jeff Law <l...@redhat.com> wrote:
> So if I'm understanding the situation correctly, with this new version
> behaviour for non-concatenated tokens is preserved which was the only
> behaviour regression in the prior patch, right?
The new version will also handle most escape sequences correctly and
simply preserve the current location for those that are not handled.
> Thus, this version of the patch is strictly an improvement (points to the
> issue within the format string rather than to the start of the string).
> Right?
I hope so :)
> I don't particularly like file scoped "offset_is_invalid" variable. It
> appears that it's only set within check_format_arg, but it's used from a
> variety of other locations via location_from_offset. Given the current
> structure of the code, alternatives would be even uglier.
This comes from the previous version of the patch, but it is not
necessary anymore, since before using an offset, we try to read the
string at the location, and if there is no string, the offset is zero.
The variable is set here:
if (TREE_CODE (format_tree) == VAR_DECL
&& TREE_CODE (TREE_TYPE (format_tree)) == ARRAY_TYPE
&& (array_init = decl_constant_value (format_tree)) != format_tree
&& TREE_CODE (array_init) == STRING_CST)
{
/* Extract the string constant initializer. Note that this may include
a trailing NUL character that is not in the array (e.g.
const char a[3] = "foo";). */
array_size = DECL_SIZE_UNIT (format_tree);
format_tree = array_init;
offset_is_invalid = true;
}
to handle this case:
void foo()
{
const char a[] = " %d ";
__builtin_printf(a, 0.5);
}
in such a case, the location we get is the one of the use of 'a', thus
we cannot get at the actual string " %d " to find an offset. Thus, it
preserves the current (not ideal) behavior.
OK with offset_is_invalid removed after regtesting?
Cheers,
Manuel.
