[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Bernd Edlinger changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #37 from Bernd Edlinger --- fixed.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #36 from Bernd Edlinger --- Author: edlinger Date: Wed Feb 27 20:14:55 2019 New Revision: 269264 URL: https://gcc.gnu.org/viewcvs?rev=269264=gcc=rev Log: 2019-02-27 Bernd Edlinger PR rtl-optimization/89490 * varasm.c (get_block_for_section): Bail out for mergeable sections. (default_use_anchors_for_symbol_p, output_object_block): Assert the block section is not mergeable. Modified: trunk/gcc/ChangeLog trunk/gcc/varasm.c
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #35 from Jakub Jelinek --- (In reply to Jakub Jelinek from comment #34) > Passed bootstrap/regtest on powerpc64le-linux, on powerpc64-linux passed > bootstrap, regtest still ongoing. Successfully regtested (-m32/-m64) on powerpc64-linux as well.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #34 from Jakub Jelinek ---
(In reply to Jakub Jelinek from comment #33)
> I've started bootstrap/regtest of this on powerpc64{,le}-linux (on
> CompileFarm).
Passed bootstrap/regtest on powerpc64le-linux, on powerpc64-linux passed
bootstrap, regtest still ongoing.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #33 from Jakub Jelinek ---
I've started bootstrap/regtest of this on powerpc64{,le}-linux (on
CompileFarm).
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Bernd Edlinger changed: What|Removed |Added Attachment #45833|0 |1 is obsolete|| --- Comment #32 from Bernd Edlinger --- Created attachment 45834 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45834=edit proposed patch addressed review comments. I'll bootstrap/regtest that once more and send it to the ML in the afternoon. Thanks everybody!
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #31 from Jakub Jelinek --- (In reply to Bernd Edlinger from comment #30) > (In reply to Jakub Jelinek from comment #29) > > Either the > >/* Don't use anchors for mergeable sections. The linker might move > > the objects around. */ > > comment should be dropped, or the two comments in > > default_use_anchors_for_symbol_p should be merged into one. > > I'd drop that comment, since "linker might move objects around" is > in the big comment now. Ok. > Is the other assert clear enough or does it also need a comment? I think it is ok as is.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #30 from Bernd Edlinger --- (In reply to Jakub Jelinek from comment #29) > Either the >/* Don't use anchors for mergeable sections. The linker might move > the objects around. */ > comment should be dropped, or the two comments in > default_use_anchors_for_symbol_p should be merged into one. I'd drop that comment, since "linker might move objects around" is in the big comment now. Is the other assert clear enough or does it also need a comment?
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #29 from Jakub Jelinek --- Either the /* Don't use anchors for mergeable sections. The linker might move the objects around. */ comment should be dropped, or the two comments in default_use_anchors_for_symbol_p should be merged into one.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Bernd Edlinger changed: What|Removed |Added Attachment #45832|0 |1 is obsolete|| --- Comment #28 from Bernd Edlinger --- Created attachment 45833 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45833=edit proposed patch addressed review comments.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #27 from Bernd Edlinger --- (In reply to rsand...@gcc.gnu.org from comment #24) > > I think we should also remove the test in default_use_anchors_for_symbol_p, > since: > > sect = SYMBOL_REF_BLOCK (symbol)->sect; > > will surely fault if the block is null. Agree the patch looks good > with those changes. > The reason why there is no segfault here is the call site in expmed.c: || SYMBOL_REF_BLOCK (base) == NULL || !targetm.use_anchors_for_symbol_p (base)) But I am okay with adding a checking assert as suggested by Jakub. (messages crossed mid-air, so I will update my patch a second time)
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Bernd Edlinger changed: What|Removed |Added Attachment #45830|0 |1 is obsolete|| --- Comment #26 from Bernd Edlinger --- Created attachment 45832 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45832=edit proposed patch updated comment.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #25 from Jakub Jelinek --- (In reply to rsand...@gcc.gnu.org from comment #24) > I think we should also remove the test in default_use_anchors_for_symbol_p, > since: > > sect = SYMBOL_REF_BLOCK (symbol)->sect; > > will surely fault if the block is null. Agree the patch looks good > with those changes. Or add gcc_checking_assert in there as well (again with a comment that get_block_for_section should have not created the block for those sections). > The idea was that the block infrastructure could also be used > at some future time to organise decls into a specific order, > e.g. to promote locality. Obviously that hasn't happened yet, > but I think we should keep the SECTION_SMALL stuff as-is and > only exclude decls that can't be put into blocks for correctness > reasons. Ack.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #24 from rsandifo at gcc dot gnu.org --- (In reply to Jakub Jelinek from comment #23) > Comment on attachment 45830 [details] > proposed patch > > Please update the get_block_for_section function comment accordingly: > > /* Return the object_block structure for section SECT. Create a new >structure if we haven't created one already. Return null if SECT >itself is null. */ > > This should say that we return NULL also for mergeable sections and why > - that section anchors can't be used in mergeable sections anyway, because > the linker might move objects around, and that using the object blocks > infrastructure in that case is both a waste and maintenance burden. I think we should also remove the test in default_use_anchors_for_symbol_p, since: sect = SYMBOL_REF_BLOCK (symbol)->sect; will surely fault if the block is null. Agree the patch looks good with those changes. > Given: > /* Don't use anchors for small data sections. The small data register > acts as an anchor for such sections. */ > if (sect->common.flags & SECTION_SMALL) > return false; > we could do the same for SECTION_SMALL, but I'm not suggesting that ATM. The idea was that the block infrastructure could also be used at some future time to organise decls into a specific order, e.g. to promote locality. Obviously that hasn't happened yet, but I think we should keep the SECTION_SMALL stuff as-is and only exclude decls that can't be put into blocks for correctness reasons. In general, the section anchor/section block stuff could probably be cleaned up a lot now that unit-at-a-time is the only supported mode. (That wasn't true when it was originally written.)
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #23 from Jakub Jelinek --- Comment on attachment 45830 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45830 proposed patch Please update the get_block_for_section function comment accordingly: /* Return the object_block structure for section SECT. Create a new structure if we haven't created one already. Return null if SECT itself is null. */ This should say that we return NULL also for mergeable sections and why - that section anchors can't be used in mergeable sections anyway, because the linker might move objects around, and that using the object blocks infrastructure in that case is both a waste and maintenance burden. Given: /* Don't use anchors for small data sections. The small data register acts as an anchor for such sections. */ if (sect->common.flags & SECTION_SMALL) return false; we could do the same for SECTION_SMALL, but I'm not suggesting that ATM.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #22 from Bernd Edlinger ---
(In reply to Jakub Jelinek from comment #20)
> I don't think users would appreciate the change in behavior where constant
> and string merging would stop to work completely.
>
> I've successfully bootstrapped/regtested the #c15 patch on
> powerpc64{,le}-linux and it looks good to me.
Thanks Jakub,
It is not surprising that Alan hit the assert because when he modified the
patch and moved it the get_block_for_decl, that enabled mergable constants
to be inside a block, there are not DECLs for.
But I would like to pick up his idea to add an assertion as attachment #45830
I have bootstrapped that successfully, and built
and arm-linux-gnueabihf cross compiler (arm has section anchors IIRC).
I would be grateful if you could give it a try on you powerpc machine.
Bernd.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #21 from Bernd Edlinger --- Created attachment 45830 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45830=edit proposed patch
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #20 from Jakub Jelinek ---
I don't think users would appreciate the change in behavior where constant and
string merging would stop to work completely.
I've successfully bootstrapped/regtested the #c15 patch on powerpc64{,le}-linux
and it looks good to me.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #19 from Alan Modra --- Created attachment 45829 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45829=edit Prevent use of merge sections when -fsection-anchors This isn't particularly elegant, but survives bootstrap and regtest on powerpc64le-linux. Companion to this patch is an rs6000 backend one that only enables -fsection-anchors by default when -mcmodel=small.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #18 from Alan Modra --- The assertion triggered in multiple places when compiling various libgcc2.c pieces, and dfp-bit.c.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #17 from Alan Modra --- The correct place for comment #15 patch is get_block_for_decl, I think. I'm bootstrapping such a patch along with an assert in output_object_block that we don't have a merge section.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Jakub Jelinek changed: What|Removed |Added CC||rsandifo at gcc dot gnu.org --- Comment #16 from Jakub Jelinek --- If that works, I'm all for it. CCing Richard Sandiford as the original author of -fsection-anchors if he remembers why this hasn't been done for SECTION_MERGE sections from the beginning. default_use_anchors_for_symbol_p starts with /* Don't use anchors for mergeable sections. The linker might move the objects around. */ sect = SYMBOL_REF_BLOCK (symbol)->sect; if (sect->common.flags & SECTION_MERGE) return false; and seems all other targetm.use_anchors_for_symbol_p hooks just return false in a few extra cases than default_use_anchors_for_symbol_p, never return true when it doesn't. So indeed, I don't see advantages in using SYMBOL_REF_BLOCK for SECTION_MERGE section symbols.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #15 from Bernd Edlinger --- I wonder if this would also work? At least for simple test cases that seems to be fine. --- varasm.c.orig 2019-01-25 17:57:32.0 +0100 +++ varasm.c2019-02-26 22:03:39.753325517 +0100 @@ -373,6 +373,9 @@ get_block_for_section (section *sect) if (sect == NULL) return NULL; + if (sect->common.flags & SECTION_MERGE) +return NULL; + object_block **slot = object_block_htab->find_slot_with_hash (sect, hash_section (sect), INSERT);
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #14 from Bernd Edlinger --- (In reply to Jakub Jelinek from comment #13) > > Well, it matches what output_constant does: > case STRING_CST: > thissize = (unsigned HOST_WIDE_INT)TREE_STRING_LENGTH (exp); > if (merge_strings > && (thissize == 0 > || TREE_STRING_POINTER (exp) [thissize - 1] != '\0')) > thissize++; > (using MAX just in case there is already some extra padding in the length or > whatever else). Ah, well. Just FYI, this check in output_constant if (size == 0 || flag_syntax_only) return size; prevents zero-padding of strings with STRING_LENGTH==0 && DECL_SIZE==0. But due to the check in mergeable_string_section && (len = int_size_in_bytes (TREE_TYPE (decl))) > 0 && TREE_STRING_LENGTH (decl) == len) we should not get size==0 or thissize == 0 in a merge_strings section. So it should not make a difference what happens with thissize==0.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #13 from Jakub Jelinek --- (In reply to Bernd Edlinger from comment #11) > Instead of: > > if (thissize == 0 > || TREE_STRING_POINTER (str) [thissize - 1] != '\0') > size = MAX (size, thissize + 1); > > maybe: > > if (thissize != 0 > && TREE_STRING_POINTER (str) [thissize - 1] != '\0') > size = MAX (size, thissize + 1); Well, it matches what output_constant does: case STRING_CST: thissize = (unsigned HOST_WIDE_INT)TREE_STRING_LENGTH (exp); if (merge_strings && (thissize == 0 || TREE_STRING_POINTER (exp) [thissize - 1] != '\0')) thissize++; (using MAX just in case there is already some extra padding in the length or whatever else).
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #12 from Bernd Edlinger --- > These should not go into mergeable sections. I mean: These do not go into mergeable sections.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #11 from Bernd Edlinger --- I agree, that it would be better to not put any mergeable things in a block object. If section anchors are ever used on a string constant, it is going to fail. A constant with size = 0 is possible, for instance in Ada an empty string really empty. These should not go into mergeable sections. output_constant ignores the merge_strings parameter, therefore I would not assume there is a zero termination Instead of: if (thissize == 0 || TREE_STRING_POINTER (str) [thissize - 1] != '\0') size = MAX (size, thissize + 1); maybe: if (thissize != 0 && TREE_STRING_POINTER (str) [thissize - 1] != '\0') size = MAX (size, thissize + 1);
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #10 from Jakub Jelinek ---
I think there is a serious flaw that the section anchor infrastructure is used
at all for the SECTION_MERGE sections, one really must not use any kind of
section anchors for those sections, there is no guarantees that the layout made
by the compiler will match what the linker will do in the end (between
different objects in the section that is, within a single object it has to
stay).
If that isn't done and we for some strange reason must still go through the
block stuff, I think we should do for something like following, where we'll
account for the artificially added trailing zeros.
--- gcc/varasm.c.jj 2019-02-22 09:30:19.133180850 +0100
+++ gcc/varasm.c2019-02-26 16:42:11.699848053 +0100
@@ -7452,7 +7452,7 @@ place_block_symbol (rtx symbol)
struct constant_descriptor_rtx *desc;
unsigned int alignment;
struct object_block *block;
- tree decl;
+ tree decl = NULL_TREE;
gcc_assert (SYMBOL_REF_BLOCK (symbol));
if (SYMBOL_REF_BLOCK_OFFSET (symbol) >= 0)
@@ -7511,6 +7511,20 @@ place_block_symbol (rtx symbol)
/* Calculate the object's offset from the start of the block. */
block = SYMBOL_REF_BLOCK (symbol);
+ if ((block->sect->common.flags & SECTION_MERGE)
+ && (block->sect->common.flags & SECTION_STRINGS)
+ && decl
+ && DECL_INITIAL (decl)
+ && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+{
+ tree str = DECL_INITIAL (decl);
+ unsigned HOST_WIDE_INT thissize = TREE_STRING_LENGTH (str);
+ if ((thissize == 0
+ || TREE_STRING_POINTER (str) [thissize - 1] != '\0')
+ && size == thissize)
+ size++;
+}
+
mask = alignment / BITS_PER_UNIT - 1;
offset = (block->size + mask) & ~mask;
SYMBOL_REF_BLOCK_OFFSET (symbol) = offset;
@@ -7639,6 +7653,8 @@ output_object_block (struct object_block
/* Output the objects themselves. */
offset = 0;
+ bool merge_strings = ((block->sect->common.flags & SECTION_MERGE)
+ && (block->sect->common.flags & SECTION_STRINGS));
FOR_EACH_VEC_ELT (*block->objects, i, symbol)
{
/* Move to the object's offset, padding with zeros if necessary. */
@@ -7657,9 +7673,18 @@ output_object_block (struct object_block
HOST_WIDE_INT size;
decl = SYMBOL_REF_DECL (symbol);
assemble_constant_contents (DECL_INITIAL (decl), XSTR (symbol, 0),
- DECL_ALIGN (decl), false);
+ DECL_ALIGN (decl), merge_strings);
size = get_constant_size (DECL_INITIAL (decl));
+ if (merge_strings
+ && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+ {
+ tree str = DECL_INITIAL (decl);
+ HOST_WIDE_INT thissize = TREE_STRING_LENGTH (str);
+ if (thissize == 0
+ || TREE_STRING_POINTER (str) [thissize - 1] != '\0')
+ size = MAX (size, thissize + 1);
+ }
offset += size;
if ((flag_sanitize & SANITIZE_ADDRESS)
&& TREE_CODE (DECL_INITIAL (decl)) == STRING_CST
@@ -7674,8 +7699,18 @@ output_object_block (struct object_block
{
HOST_WIDE_INT size;
decl = SYMBOL_REF_DECL (symbol);
- assemble_variable_contents (decl, XSTR (symbol, 0), false, false);
+ assemble_variable_contents (decl, XSTR (symbol, 0), false,
+ merge_strings);
size = tree_to_uhwi (DECL_SIZE_UNIT (decl));
+ if (merge_strings
+ && TREE_CODE (DECL_INITIAL (decl)) == STRING_CST)
+ {
+ tree str = DECL_INITIAL (decl);
+ HOST_WIDE_INT thissize = TREE_STRING_LENGTH (str);
+ if (thissize == 0
+ || TREE_STRING_POINTER (str) [thissize - 1] != '\0')
+ size = MAX (size, thissize + 1);
+ }
offset += size;
if ((flag_sanitize & SANITIZE_ADDRESS)
&& asan_protect_global (decl))
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #9 from Bernd Edlinger ---
> And, what do you find wrong on the alignment?
In the case of the artificially zero terminated strings,
the .zero is now wrong, and they can actually screw up the
necessary alignment.
Maybe the easiest way out would be simply not try to put
non-zero terminated strings in merge sections, when the
-fsection-anchors is in effect?
--- varasm.c.orig 2019-01-25 17:57:32.0 +0100
+++ varasm.c2019-02-26 09:48:48.061315685 +0100
@@ -838,7 +838,8 @@
if (j == unit)
break;
}
- if (i == len - unit || (unit == 1 && i == len))
+ if (i == len - unit || (unit == 1 && i == len &&
+ !flag_section_anchors))
{
sprintf (name, "%s.str%d.%d", prefix,
modesize / 8, (int) (align / 8));
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #8 from Jakub Jelinek --- (In reply to Bernd Edlinger from comment #5) > Adding zero bytes after each string constant makes no sense IMHO, > since the linker will merge the constants, and so aligning the > constants with .zero does probably not work, but the should be > benign, except for that the alignment of the string constants. The linker string merging code is not working with the asm directives, it doesn't really care what part of the string comes from .string or .ascii and what part comes from .zero directives. And, what do you find wrong on the alignment? The section name indicates 8 byte alignment and .align 3 directive (which is like .balign 8 on this target) says the alignment of the strings is 8 byte, so all the strings (including a single terminating '\0') must be padded to multiples of 8 bytes by zeros.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 --- Comment #7 from Bernd Edlinger --- (In reply to Alan Modra from comment #6) > The zero bytes are added by the -fsection-anchors code. They used to align > the next object. Now, the number of zero bytes is wrong (in cases where we > used to have an unterminated string), *and* gcc's calculation of the offset > to the object within the section-anchor block is wrong. > > Misaligned strings will mean poorer performance on some targets. > Miscalculating the offset will result in wrong-code errors when it results > in too many objects being placed into a block. Well, I see, the alignment is not right, but even if the strings are near to each other, they will be moved to other segments, if the string constant was already used in another module. Are there cases where the string constants need to be near the section anchor, what is that and how does it look like in an assembly file?
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Alan Modra changed: What|Removed |Added Priority|P3 |P1 Status|UNCONFIRMED |NEW Last reconfirmed|2019-02-25 00:00:00 |2019-02-26 Target Milestone|--- |9.0 Ever confirmed|0 |1
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Alan Modra changed: What|Removed |Added Priority|P1 |P3 Status|NEW |UNCONFIRMED Target Milestone|9.0 |--- Ever confirmed|1 |0 --- Comment #6 from Alan Modra --- The zero bytes are added by the -fsection-anchors code. They used to align the next object. Now, the number of zero bytes is wrong (in cases where we used to have an unterminated string), *and* gcc's calculation of the offset to the object within the section-anchor block is wrong. Misaligned strings will mean poorer performance on some targets. Miscalculating the offset will result in wrong-code errors when it results in too many objects being placed into a block.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #5 from Bernd Edlinger ---
The patch should probably work, and a powerpc cross fixes the test case.
At least bootstrap and reg-test on x86_64-pc-linux-gnu is fine,
but that proves not too much.
When I look at the merge-all-constants-2.c test case, I think
the mergeable string secrtion looks bogus (unrelated to this):
$ cat merge-all-constants-2.c
/* { dg-do compile } */
/* { dg-require-effective-target string_merging } */
/* { dg-options "-w -O2 -fmerge-all-constants" } */
const char str1[36] = "0123456789abcdefghijklmnopqrstuvwxyz";
const char str2[37] = "0123456789abcdefghijklmnopqrstuvwxyz";
const char str3[10] = "0123456789abcdefghijklmnopqrstuvwxyz";
/* { dg-final { scan-assembler-not "\\.rodata\[\n\r\]" } } */
$ powerpc64-unknown-linux-gcc -O2 -S merge-all-constants-2.c
merge-all-constants-2.c:7:23: warning: initializer-string for array of chars is
too long
7 | const char str3[10] = "0123456789abcdefghijklmnopqrstuvwxyz";
| ^~
$ cat merge-all-constants-2.s
.file "merge-all-constants-2.c"
.machine power4
.section".text"
.globl str3
.globl str2
.globl str1
.section.rodata.str1.8,"aMS",@progbits,1
.align 3
.type str3, @object
.size str3, 10
str3:
.string "0123456789"
.zero 6
.type str2, @object
.size str2, 37
str2:
.string "0123456789abcdefghijklmnopqrstuvwxyz"
.zero 3
.type str1, @object
.size str1, 36
str1:
.string "0123456789abcdefghijklmnopqrstuvwxyz"
.ident "GCC: (GNU) 9.0.1 20190222 (experimental)"
Adding zero bytes after each string constant makes no sense IMHO,
since the linker will merge the constants, and so aligning the
constants with .zero does probably not work, but the should be
benign, except for that the alignment of the string constants.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490
--- Comment #4 from Bernd Edlinger ---
funny, how I managed to overlook this...
--- varasm.c.orig 2019-01-25 17:57:32.0 +0100
+++ varasm.c2019-02-25 13:13:55.652051780 +0100
@@ -7634,6 +7634,8 @@
/* Output the objects themselves. */
offset = 0;
+ bool merge_strings = (block->sect->common.flags & SECTION_MERGE)
+&& (block->sect->common.flags & SECTION_STRINGS);
FOR_EACH_VEC_ELT (*block->objects, i, symbol)
{
/* Move to the object's offset, padding with zeros if necessary. */
@@ -7652,7 +7654,7 @@
HOST_WIDE_INT size;
decl = SYMBOL_REF_DECL (symbol);
assemble_constant_contents (DECL_INITIAL (decl), XSTR (symbol, 0),
- DECL_ALIGN (decl), false);
+ DECL_ALIGN (decl), merge_strings);
size = get_constant_size (DECL_INITIAL (decl));
offset += size;
@@ -7669,7 +7671,8 @@
{
HOST_WIDE_INT size;
decl = SYMBOL_REF_DECL (symbol);
- assemble_variable_contents (decl, XSTR (symbol, 0), false, false);
+ assemble_variable_contents (decl, XSTR (symbol, 0), false,
+ merge_strings);
size = tree_to_uhwi (DECL_SIZE_UNIT (decl));
offset += size;
if ((flag_sanitize & SANITIZE_ADDRESS)
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Jakub Jelinek changed: What|Removed |Added Priority|P3 |P1 Status|UNCONFIRMED |NEW Last reconfirmed||2019-02-25 CC||jakub at gcc dot gnu.org Target Milestone|--- |9.0 Ever confirmed|0 |1 --- Comment #3 from Jakub Jelinek --- Started with r264850.
[Bug rtl-optimization/89490] [9 Regression] char array constant put in string merge section
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89490 Alan Modra changed: What|Removed |Added CC||bernd.edlinger at hotmail dot de Summary|char array constant put in |[9 Regression] char array |string merge section|constant put in string ||merge section --- Comment #2 from Alan Modra --- Putting the array initialiser in a STRING_CST conflicts with varasm.c:categorize_decl_for_section returning SECCAT_RODATA_MERGE_STR for STRING_CST.
