Thanks for looking at this.

Segher Boessenkool <[EMAIL PROTECTED]> 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

Reply via email to