This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR55033: Fix


On Sat, Mar 30, 2013 at 06:29:36PM -0400, David Edelsohn wrote:
> How can we make progress to get this patch committed on trunk, 4.8 and 4.7?

I have OKs for the config/i386/winnt.c and config/rs6000/rs6000.c
parts.  I just need someone who is authorized to review patches to
varasm.c, and is willing to risk their reputation to at least comment
on the patch.  Even a loud "NO" would be better than silence.  As it
stands, I'm sorry I offered the patch, even though I still believe the
patch is correct from a design viewpoint, better than Sebastian's
patch that just tackled the specific case of ".sdata2".  His patch is
obviously easier to review, we'd be OK on powerpc..

Let's try again with all the information in one place, and perhaps a
better explanation.  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55033
is about a problem on powerpc eabi, where gcc generates wrong section
flags (SECTION_WRITE) for an array initializer that should live in
.sdata2, a read-only section, and then hits an internal consistency
check because gcc gets the correct flags for .sdata2 in other cases.
See attached C testcase.

Sebastian offered a patch to default_section_type_flags that would
correct the flags, but his approach is going back to the mess we had
before Richard Henderson gave us categorize_decl_for_section.  I made
the comment that we ought to use categorize_decl_for_section for
selecting the section flags, if we've used categorize_decl_for_section
to select the section name.  Specifically, default_elf_select_section
should allow this to happen.  rth agreed with this design approach in
http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02487.html, but that 
particular patch of mine was flawed, and I fixed a followup PR in a
non-ideal way.

So http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02172.html is actually
a fix for a very old patch of mine.  I can well understand a reviewer
looking at the patch and scratching their heads a little.  Not so much
due to the latest patch, but because existing code in this area is
suspicious.  For instance, you might wonder why it is correct to have
  if (decl && !DECL_P (decl))
    decl = NULL_TREE;
before calling get_section().  The answer is that get_section() is not
prepared to handle !DECL_P trees when reporting errors.  Arguably it
should be modified to do that.

-- 
Alan Modra
Australia Development Lab, IBM
/* { dg-do compile { target powerpc*-*-eabi* powerpc*-*-elf* powerpc*-*-linux* } } */
/* { dg-require-effective-target ilp32 } */
/* { dg-options "-mcpu=8540 -msoft-float -meabi -msdata" } */

void f(void);

struct s {
  int *p;
  int *q;
};

extern int a;

extern const struct s c;

const struct s c = { &a, 0 };

void f(void)
{
  char buf[4] = { 0, 1, 2, 3 };
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]