[PATCH 2/2] PR preprocessor/69126: avoid comparing ad-hoc and non-ad-hoc locations

Jeff Law law@redhat.com
Mon Feb 22 18:41:00 GMT 2016


On 02/20/2016 09:46 AM, David Malcolm wrote:
> Comment #18 of PR preprocessor/69126 reported a difficult-to-reproduce
> re-occurrence of that bug, where attempts to suppress
>    -Wdeprecated-declarations
> via a _Pragma could fail.
>
> The root cause is a bug in linemap_compare_locations when comparing
> certain macro expansions with certain non-macro expansions.
>
> (gdb) call inform (pre, "pre")
> test.cc:8:16: note: pre
>   #define IGNORE _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
>                  ^
> test.cc:8:16: note: in definition of macro ‘IGNORE’
>   #define IGNORE _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
>                  ^~~~~~~
> (gdb) call inform (post, "post")
> test.cc:12:5: note: post
>       f();
>       ^
> After macro expansion, we have (at the end of linemap_compare_locations):
> (gdb) p /x l0
> $13 = 0x800101ec
> (gdb) p /x l1
> $14 = 0x50684c05
>
> and hence:
>
> (gdb) p /x l1 - l0
> $23 = 0xd0674a19
>
> it's effectively negative, and so "before_p" is false; it's erroneously
> treating the _Pragma as if it were *after* the diagnostic (and hence
> it doesn't affect it).
>
> But this is wrong: l0 is an ad-hoc loc, whereas l1 is a non-ad-hoc loc.
>
> It's clearly insane to do pure numeric comparisons of ad-hoc
> locations with non-ad-hoc locations, since doing so will make the
> latter always be "after" the former.  The fix is simple: resolve
> ad-hoc locations at the end of linemap_compare_locations.
>
> For this bug to occur, we need a location for the macro name that's an
> ad-hoc location, and a location for the diagnostic that's *not* an
> ad-hoc location.
>
> The reason it triggered for the reporter of comment #18 of the PR is
> that the sheer quantity of code in his reproducer meant that both
> locations in question were above LINE_MAP_MAX_LOCATION_WITH_PACKED_RANGES
> (but below LINE_MAP_MAX_LOCATION_WITH_COLS), so range-packing was
> disabled, in particular for the "IGNORE" macro, giving an ad-hoc
> location for the _Pragma; in contrast, the diagnostic a single
> character, and thus used a non-ad-hoc location.
>
> The attached patch fixes the issue, and adds test coverage, via a pair
> of test cases c-c++-common/pr69126-2-{long|short}.c, where the "long"
> version of the test case uses a macro name that's >=32 characters (thus
> forcing the use of an ad-hoc location), which reproduced the issue.
>
> Given that this adds calls to get_location_from_adhoc_loc to the end
> of linemap_compare_locations, I went ahead and removed the hand-inlined
> copies from the top of the function (but I can avoid that change if
> that's too much for stage 4).
>
> This also fixes the xfail in pr69543-1.c, as
> "YY_IGNORE_MAYBE_UNINITIALIZED_BEGIN" is longer than 31 chars and
> was thus affected by this.
>
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu in
> combination with the previous patch.
>
> OK for trunk in stage 4?
>
> gcc/testsuite/ChangeLog:
> 	PR preprocessor/69126
> 	PR preprocessor/69543
> 	* c-c++-common/pr69126-2-long.c: New test.
> 	* c-c++-common/pr69126-2-short.c: New test.
> 	* c-c++-common/pr69543-1.c: Remove xfail.
>
> libcpp/ChangeLog:
> 	PR preprocessor/69126
> 	PR preprocessor/69543
> 	* line-map.c (linemap_compare_locations): At the function top,
> 	replace inlined bodies of get_location_from_adhoc_loc with calls
> 	to get_location_from_adhoc_loc.  Add a pair of calls to
> 	get_location_from_adhoc_loc at the bottom of the function, to
> 	avoid meaningless comparisons of ad-hoc and non-ad-hoc locations.
OK.  Thanks for tracking this down.

jeff



More information about the Gcc-patches mailing list