<div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Kasun,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Great that you have already found a fix for your issue. The commitSection method would only be called if the output section actually contains an input section. So for "empty" output sections the "noload" was not having any effect. This is basically what D76981 fixes.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Thanks.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Regards,</div><div class="gmail_default" style="font-size:small">Andrew</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 30 Mar 2020 at 12:57, Kasun Fernando <<a href="mailto:kasunf@blackmagicdesign.com">kasunf@blackmagicdesign.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div>
    <p>Hi Andrew,</p>
    <p>Thanks for the background and context.<br>
      <br>
    </p>
    <div class="gmail_default" style="font-size:small">"In your issue,
      just to clarify, is the ELF output from LLD also "large", or is it
      just the output from the llvm-objcopy operating on that ELF that
      is "large"? Do you have a simple sample to demonstrate this
      issue?"<br>
      <br>
      <font size="+1">The ELF size is actually smaller, compared to what
        was generated from LLVM 7.x. (~900Kb vs ~250Kb)</font></div>
    <div class="gmail_default" style="font-size:small"><font size="+1"><br>
      </font></div>
    <div class="gmail_default" style="font-size:small"><font size="+1">When
        we run llvm-objcopy -O Binary blah.elf blah.bin, it would
        generate a 400Mb binary file.</font></div>
    <div class="gmail_default" style="font-size:small"> <br>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small">I presume the
      `noload` flag used in `InputSection` is not properly enforced
      throughout the code. CommitSection is not applied to .heap and
      .stack section as I can recall during my debug....</div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small">
      <pre style="margin:0px;padding:0px;white-space:pre-wrap"><pre style="line-height:125%"><span style="color:rgb(0,128,0);font-weight:bold">.stack</span> <span style="color:rgb(160,160,0)">(NOLOAD)</span> : <span style="color:rgb(0,128,0)">ALIGN(4)</span> <span style="color:rgb(0,128,0)">{</span>
        <span style="color:rgb(0,128,0)">_StackLow</span> = <span style="color:rgb(0,128,0)">.;</span>
        <span style="color:rgb(0,128,0)">.</span> <span style="color:rgb(0,128,0)">+</span>= <span style="color:rgb(0,128,0)">_STACK_SIZE;</span> <span style="color:rgb(0,128,0)">/*</span> <span style="color:rgb(0,128,0)">Multiple</span> <span style="color:rgb(0,128,0)">of</span> <span style="color:rgb(0,128,0)">4,</span> <span style="color:rgb(0,128,0)">asserted</span> <span style="color:rgb(0,128,0)">above.</span> <span style="color:rgb(0,128,0)">*/</span>
        <span style="color:rgb(0,128,0)">_StackHigh</span> = <span style="color:rgb(0,128,0)">.;</span>

        <span style="color:rgb(0,128,0)">_StackLowIrq</span> = <span style="color:rgb(0,128,0)">.;</span>
        <span style="color:rgb(0,128,0)">.</span> <span style="color:rgb(0,128,0)">+</span>= <span style="color:rgb(0,128,0)">_IRQ_STACK_SIZE;</span> <span style="color:rgb(0,128,0)">/*</span> <span style="color:rgb(0,128,0)">Multiple</span> <span style="color:rgb(0,128,0)">of</span> <span style="color:rgb(0,128,0)">4,</span> <span style="color:rgb(0,128,0)">asserted</span> <span style="color:rgb(0,128,0)">above.</span> <span style="color:rgb(0,128,0)">*/</span>
        <span style="color:rgb(0,128,0)">_StackHighIrq</span> = <span style="color:rgb(0,128,0)">.;</span>
    <span style="color:rgb(0,128,0)">}</span> <span style="color:rgb(0,128,0)">></span> <span style="color:rgb(0,128,0)">Memory_SRAM</span>

<span style="color:rgb(0,128,0)">....</span>
<span style="color:rgb(0,128,0)">/*</span> <span style="color:rgb(0,128,0)">Finally</span> <span style="color:rgb(0,128,0)">we</span> <span style="color:rgb(0,128,0)">do</span> <span style="color:rgb(0,128,0)">the</span> <span style="color:rgb(0,128,0)">heap</span> <span style="color:rgb(0,128,0)">*/</span>
    <span style="color:rgb(0,128,0)">.heap</span> <span style="color:rgb(160,160,0)">(NOLOAD)</span> : <span style="color:rgb(0,128,0)">ALIGN(4)</span> <span style="color:rgb(0,128,0)">{</span>
        <span style="color:rgb(0,128,0)">ASSERT((_HEAP_SIZE</span> <span style="color:rgb(0,128,0)">%</span> <span style="color:rgb(102,102,102)">4</span> == <span style="color:rgb(0,128,0)">0),</span> <span style="color:rgb(186,33,33)">"Error: _HEAP_SIZE must be a multiple of 4."</span><span style="color:rgb(0,128,0)">);</span>
        <span style="color:rgb(0,128,0)">_HeapLow</span> = <span style="color:rgb(0,128,0)">.;</span>
        <span style="color:rgb(0,128,0)">/*</span> <span style="color:rgb(0,128,0)">Use</span> <span style="color:rgb(0,128,0)">as</span> <span style="color:rgb(0,128,0)">much</span> <span style="color:rgb(0,128,0)">space</span> <span style="color:rgb(0,128,0)">as</span> <span style="color:rgb(0,128,0)">we</span> <span style="color:rgb(0,128,0)">can</span> <span style="color:rgb(0,128,0)">for</span> <span style="color:rgb(0,128,0)">the</span> <span style="color:rgb(0,128,0)">heap</span> <span style="color:rgb(0,128,0)">*/</span>
        <span style="color:rgb(0,128,0)">.</span> = <span style="color:rgb(0,128,0)">ORIGIN(Memory_SRAM)</span> <span style="color:rgb(0,128,0)">+</span> <span style="color:rgb(0,128,0)">LENGTH(Memory_SRAM);</span>
        <span style="color:rgb(0,128,0)">_HeapHigh</span> = <span style="color:rgb(0,128,0)">.;</span>
        <span style="color:rgb(0,128,0)">ASSERT((_HeapLow</span> <span style="color:rgb(0,128,0)">+</span> <span style="color:rgb(0,128,0)">_HEAP_SIZE</span> <span style="color:rgb(0,128,0)"><</span> <span style="color:rgb(0,128,0)">_HeapHigh),</span> <span style="color:rgb(186,33,33)">"Error: Not enough space for the heap: data section too big."</span><span style="color:rgb(0,128,0)">);</span>
    <span style="color:rgb(0,128,0)">}</span> <span style="color:rgb(0,128,0)">></span> <span style="color:rgb(0,128,0)">Memory_SRAM

</span></pre></pre>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
      <pre style="box-sizing:inherit;margin:4px 0px;padding:8px;font-size:12px;line-height:1.50001;font-variant-ligatures:none;white-space:pre-wrap;word-break:normal;border-radius:4px;color:rgb(29,28,29);font-style:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:left;text-indent:0px;text-transform:none;word-spacing:0px;text-decoration-style:initial;text-decoration-color:initial;font-family:Monaco,Menlo,Consolas,"Courier New",monospace">void OutputSection::commitSection(InputSection *isec) {
  if (!hasInputSections) {
    // If IS is the first section to be added to this section,
    // initialize type, entsize and flags from isec.
    hasInputSections = true;
    type = isec->type;
    entsize = isec->entsize;
    flags = isec->flags;
  } else {
    // Otherwise, check if new type or flags are compatible with existing ones.
    unsigned mask = SHF_TLS | SHF_LINK_ORDER;
    if ((flags & mask) != (isec->flags & mask))
      error("incompatible section flags for " + name + "\n>>> " + toString(isec) +
            ": 0x" + utohexstr(isec->flags) + "\n>>> output section " + name +
            ": 0x" + utohexstr(flags));
    if (type != isec->type) {
      if (!canMergeToProgbits(type) || !canMergeToProgbits(isec->type))
        error("section type mismatch for " + isec->name + "\n>>> " +
              toString(isec) + ": " +
              getELFSectionTypeName(config->emachine, isec->type) +
              "\n>>> output section " + name + ": " +
              getELFSectionTypeName(config->emachine, type));
      type = SHT_PROGBITS;
    }
  }
  if (noload)
    type = SHT_NOBITS;
</pre>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small">However, having
      said that, I think the following fix (although not the cleanest
      nor addresses any shortcommings in other parts of the code)  seems
      to fix my issue. <br>
    </div>
    <div class="gmail_default" style="font-size:small"><a href="https://reviews.llvm.org/D76981" target="_blank">https://reviews.llvm.org/D76981</a></div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small">I'm also hoping
      to get involve in the LLVM development in near future.....<br>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small"><br>
    </div>
    <div class="gmail_default" style="font-size:small">regards</div>
    <div class="gmail_default" style="font-size:small">Kasun<br>
    </div>
    <p><br>
    </p>
    <div>On 30/3/20 9:50 pm, Andrew Ng wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div class="gmail_default" style="font-size:small">Hi Kasun,</div>
        <div class="gmail_default" style="font-size:small"><br>
        </div>
        <div class="gmail_default" style="font-size:small">The purpose
          of commit ccba42c7eb3cdfe7824cd4b473a9688e5738fa3a was to fix
          an issue that was causing incorrect segment file offset
          alignment for any non-empty segment that happens to start with
          a section that only contains symbols and no other content. If
          you look at the test case
          "ELF/linkerscript/symbol-only-align.test" that might help
          demonstrate the situation. This particular issue actually
          resulted in invalid ELF output.</div>
        <div class="gmail_default" style="font-size:small"><br>
        </div>
        <div class="gmail_default" style="font-size:small">I don't
          remember all the details, but at the time, this was the
          simplest fix given the code at that point. The alternatives
          would have required more significant and riskier changes, and
          it was a relatively urgent fix given that it was producing
          invalid ELF output.</div>
        <div class="gmail_default" style="font-size:small"><br>
        </div>
        <div class="gmail_default" style="font-size:small">In your
          issue, just to clarify, is the ELF output from LLD also
          "large", or is it just the output from the llvm-objcopy
          operating on that ELF that is "large"? Do you have a simple
          sample to demonstrate this issue?</div>
        <div class="gmail_default" style="font-size:small"><br>
        </div>
        <div class="gmail_default" style="font-size:small">Thank you.</div>
        <div class="gmail_default" style="font-size:small"><br>
        </div>
        <div class="gmail_default" style="font-size:small">Regards,</div>
        <div class="gmail_default" style="font-size:small">Andrew</div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Sun, 29 Mar 2020 at 22:50,
          Kasun Fernando <<a href="mailto:kasunf@blackmagicdesign.com" target="_blank">kasunf@blackmagicdesign.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi
          LLVM devs,<br>
          <br>
          <br>
            I came across an LLD bug in v 10.x where ELF parser /
          processor is <br>
          setting .PROGBITS attribute for .heap and .stack sections,
          which leads <br>
          to large binaries when we do `llvm-objcopy -o binary` to
          generate the <br>
          binary output for armv6m. (e.g. for a 57Kb elf would yield a
          ~400Mb <br>
          binary).<br>
          <br>
          This in comparison with LLVM 7.x , would produce the correct
          binary size <br>
          of 35Kb and the elf sections have NOBITS for .heap and .stack
          sections.<br>
          <br>
          <br>
          I narrowed down the problem to the following commit and the
          commits <br>
          around this....Please see below:<br>
          <br>
          <br>
          commit ccba42c7eb3cdfe7824cd4b473a9688e5738fa3a<br>
          Author: Andrew Ng <<a href="mailto:anng.sw@gmail.com" target="_blank">anng.sw@gmail.com</a>><br>
          Date:   Tue Apr 23 12:38:52 2019 +0000<br>
          <br>
               [ELF] Change default output section type to SHT_PROGBITS<br>
          <br>
               This fixes an issue where a symbol only section at the
          start of a<br>
               PT_LOAD segment, causes incorrect alignment of the file
          offset for the<br>
               start of the segment which results in the output of an
          invalid ELF.<br>
          <br>
               SHT_PROGBITS was the default output section type in the
          past.<br>
          <br>
               Differential Revision: <a href="https://reviews.llvm.org/D60131" rel="noreferrer" target="_blank">https://reviews.llvm.org/D60131</a><br>
          <br>
               llvm-svn: 358981<br>
          <br>
          <br>
          commit 5929553868ddfd3f53672253782260c2a0a52c79<br>
          Author: Fangrui Song <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>><br>
          Date:   Wed Apr 24 14:44:07 2019 +0000<br>
          <br>
               [ELF] Delete a redundant SHT_NOBITS -> SHT_PROGBITS
          after D60131<br>
          <br>
               Differential Revision: <a href="https://reviews.llvm.org/D61006" rel="noreferrer" target="_blank">https://reviews.llvm.org/D61006</a><br>
          <br>
               llvm-svn: 359099<br>
          <br>
          <br>
          What was the intention behind "Andrew Ng"'s commit? was it an
          oversight <br>
          to set PROGBITS attribute unconditionally for sections that
          are <br>
          allocation only?<br>
          <br>
          <br>
          regards<br>
          <br>
          Kasun<br>
          <br>
          <br>
        </blockquote>
      </div>
    </blockquote>
  </div>

</blockquote></div>