[PATCH] D107208: [ELF] Make dot in .tbss correct and drop threadBssOffset

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I'm a little nervous about this change as is. I think that if we only support one SHT_NOBITS SHF_TLS OutputSection then we should give an error message if there is more than one. Perhaps detect this in the PT_TLS output code. My main concern is about the the internal default (no linker script) and orphan TLS sections that don't have the .tbss. prefix. IIUC these will be placed after .tbss and will be covered by the PT_TLS program header but we will silently give these the wrong address. I'm sure these will be rare, but without an error message we may silently break some programs. I'm not as concerned about the linker script case as the onus is on the author to get the patterns right so that there are no orphans.

As a thought, could we keep with a slight modification?

  bool isTbss =
      (ctx->outSec->flags & SHF_TLS) && ctx->outSec->type == SHT_NOBITS;
  uint64_t start = isTbss ? dot + ctx->threadBssOffset : dot;
  start = alignTo(start, alignment);
  uint64_t end = start + size;
  
  if (isTbss)
    ctx->threadBssOffset = end - dot;
  
  dot = end;
  return end;

In this way we set dot, which should permit the linker script to define symbols within the same output section, but dot will then get reset by your new code

  // NOBITS TLS sections reset the location as well. Having another TLS section
  // below sec is unsupported and regarded as user error.

so the threadBssOffset should be as it was previously.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107208/new/

https://reviews.llvm.org/D107208