This is the mail archive of the gcc@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 PR31490] Re: another build failure on ppc64-linux


Thanks for looking at this.

Segher Boessenkool <segher@kernel.crashing.org> writes:
> +         /* Allow mixed writable and read-only objects in named 
> sections.  */
> +         if ((sect->common.flags & SECTION_NAMED) != 0
> +             && ((sect->common.flags ^ flags) & ~SECTION_DECLARED)
> +                == SECTION_WRITE)
> +           {
> +             sect->common.flags |= SECTION_WRITE;
> +             return sect;
> +           }
> +

The SECTION_NAMED check is redundant; this function only deals with
named sections.  FWIW, I think it would be cleaner to put:

      if (((sect->common.flags ^ flags) & SECTION_WRITE) != 0)
        sect->common.flags |= SECTION_WRITE;

before:

      if ((sect->common.flags & ~SECTION_DECLARED) != flags
	  && ((sect->common.flags | flags) & SECTION_OVERRIDE) == 0)
	{
	  /* Sanity check user variables for flag changes.  */
	  if (decl == 0)
	    decl = sect->named.decl;
	  gcc_assert (decl);
	  error ("%+D causes a section type conflict", decl);
	}

Do we want to still complain when the section has been declared
(i.e. when SECTION_DECLARED is set)?  Or do we just leave that
to the assembler?

IMO, the system_error.lo case is genuine bug; it's picking a section
for a read-only variable before marking it as read-only.  So for this
case, I think the patch is actually papering over a real problem.
(That's not an objection to the patch though.)

Richard


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