On 9/15/20 5:15 PM, Joseph Myers wrote:
On Wed, 9 Sep 2020, Martin Sebor via Gcc-patches wrote:
Ping: https://gcc.gnu.org/pipermail/gcc-patches/2020-August/552500.html
Aldy provided a bunch of comments on this patch but I'm still looking
for a formal approval.
This patch is OK.
Some testing revealed that the code has different semantics for
strings: it compares them including all nuls embedded in them,
while I think the right semantics for attributes is to only consider
strings up and including the first nul (i.e., apply the usual string
semantics). A test case for this corner case is as follows:
__attribute__ ((section ("foo\0bar"))) void f (void);
__attribute__ ((section ("foo"))) void f (void) { }
Without operand_equal_p() GCC accepts this and puts f in section
foo. With the operand_equal_p() change above, it complains:
ignoring attribute ‘section ("foo\x00bar")’ because it conflicts with
previous ‘section ("foor")’ [-Wattributes]
I would rather not change the behavior in this corner case just to
save a few lines of code. If it's thought that string arguments
to attributes (some or all) should be interpreted differently than
in other contexts it seems that such a change should be made
separately and documented in the manual.
I think that for at least the section attribute, embedded NULs are
suspect. In that case, so are wide strings, but for some attributes wide
strings should be accepted and handled sensibly (but aren't, see bug
91182).
I agree. Clang has the same "bug" as GCC would if it used
operand_equal_p().
Besides embedded nuls, other characters are problematic and cause
(sometimes confusing) assembler errors. Examples are the empty
string, embedded whitespace, or mismatched quotes. It would be
nice to do some basic validation and print a nice warning for
the most common problems.
Martin
