[clang] [lld] [llvm] Deprecate the `-fbasic-block-sections=labels` option. (PR #107494)

2024-09-13 Thread James Henderson via cfe-commits

jh7370 wrote:

Just chiming in that I happened to spot the pre-merge check failure looks 
possibly related.

https://github.com/llvm/llvm-project/pull/107494
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm] [Support] Remove raw_ostream::tie (PR #97396)

2024-07-02 Thread James Henderson via cfe-commits

jh7370 wrote:

> What was the motivation to keep this in llvm-dwarfdump?

If warnings or errors are printed by llvm-dwarfdump without this, we end up 
with corrupted output, where stderr and stdout are intermixed in a 
semi-arbitrary way. It was a while back that I made the original change mind 
you, so I don't have the concrete example in front of me any more. Looking at 
discussions in the original reviews on related commits make me think 
llvm-dwarfdump's --debug-line option was particularly badly affected: I'd be 
surprised if there aren't tests that start failing (potentially flakily, given 
that it'll depend on buffer sizes and therefore the amount of data written, 
which may be platform-specific etc) because of this change.

In practice, I wanted this enabled for all tools however, it just never 
happened.

>  Is there any other way to achieve the same goal?

Unless the additional check is causing serious issues, I'm not sure this is the 
right question (and even if it is the right question, I think it's on you as 
the person proposing to remove the change to provide an alternative - I don't 
really work on LLVM beyond reviewing these days, so I don't have the time to 
consider alternatives to work that is currently doing the job well for at least 
one initial use-case I care about). What I feel the right initial question is, 
is this check actually making a significant difference to performance in 
real-world use cases?

https://github.com/llvm/llvm-project/pull/97396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Driver] Add -Wa, options --crel and --allow-experimental-crel (PR #97378)

2024-07-02 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I've skimmed through this, and it looks okay, but I don't have enough 
experience with the clang driver to feel comfortable signing it off.

https://github.com/llvm/llvm-project/pull/97378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-07-01 Thread James Henderson via cfe-commits

https://github.com/jh7370 approved this pull request.

LGTM.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [PAC][ELF][AArch64] Encode signed GOT flag in PAuth core info (PR #96159)

2024-07-01 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

Thanks, llvm-readobj aspects look good to me (I don't feel like I can review 
the clang side).

https://github.com/llvm/llvm-project/pull/96159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-28 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

Just noting that I've taken another look through the whole thing and I have no 
new comments.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [PAC][ELF][AArch64] Encode signed GOT flag in PAuth core info (PR #96159)

2024-06-27 Thread James Henderson via cfe-commits

jh7370 wrote:

> > I'm not at all familiar with this PAuth stuff, but don't you need a test 
> > case for where the new value is set (currently they all seem to be unset, 
> > if I'm interpreting things correctly)?
> 
> @jh7370 I'm not sure if I understood your question correctly - particularly, 
> I'm not sure what does the phrase "the new value is set" mean. Could you 
> please add a bit more details in your question?
> 
> If you are talking about 
> llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s and 
> llvm/test/CodeGen/AArch64/note-gnu-property-elf-pauthabi.ll tests checking 
> version value 0x55 which does not imply signed GOT enabled, we just can't 
> test 2^8=256 combinations of flags, so we test values which look like 
> 0b10101... But I can add a test for version value 0xAA which would set 
> opposite flags compared to 0x55.

I was referring to this line from the description:

> llvm-readobj: print `PointerAuthELFGOT` or `!PointerAuthELFGOT` in version 
> description of llvm_linux platform depending on whether the flag is set.

In my opinion, if you don't test the first of those two cases, you might as 
well not have implemented behaviour for it. I'd always test "all flags set" and 
"no flags set" cases (or some variant that effectively tests this, e.g. 0xff 
and ~0xff). Of course, if it's not practical, that's fine.

https://github.com/llvm/llvm-project/pull/96159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [PAC][ELF][AArch64] Encode signed GOT flag in PAuth core info (PR #96159)

2024-06-27 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I'm not at all familiar with this PAuth stuff, but don't you need a test case 
for where the new value is set (currently they all seem to be unset, if I'm 
interpreting things correctly)?

https://github.com/llvm/llvm-project/pull/96159
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -86,6 +86,8 @@ DYNAMIC_TAG(RELRSZ, 35)  // Size of Relr relocation table.
 DYNAMIC_TAG(RELR, 36)// Address of relocation table (Relr entries).
 DYNAMIC_TAG(RELRENT, 37) // Size of a Relr relocation entry.
 
+DYNAMIC_TAG(CREL,  38)   // CREL relocation table
+

jh7370 wrote:

I still disagree with this. Is it just me? What do @smithp35 and @dwblaikie 
think?

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -4888,6 +4920,34 @@ void ELFDumper::printRelocationsHelper(const 
Elf_Shdr &Sec) {
 
 template  void ELFDumper::printDynamicRelocationsHelper() {
   const bool IsMips64EL = this->Obj.isMips64EL();
+  auto DumpCrelRegion = [&](DynRegionInfo &Region) {
+// While the size is unknown, a valid CREL has at least one byte. We can
+// check whether Addr is in bounds, and then decode CREL until the file
+// end.
+Region.Size = Region.EntSize = 1;
+if (!Region.template getAsArrayRef().empty()) {
+  const uint64_t Offset =
+  Region.Addr -
+  (const uint8_t *)ObjF.getMemoryBufferRef().getBufferStart();

jh7370 wrote:

Nit: reinterpret_cast

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -4888,6 +4920,34 @@ void ELFDumper::printRelocationsHelper(const 
Elf_Shdr &Sec) {
 
 template  void ELFDumper::printDynamicRelocationsHelper() {
   const bool IsMips64EL = this->Obj.isMips64EL();
+  auto DumpCrelRegion = [&](DynRegionInfo &Region) {

jh7370 wrote:

The lambda feels unnecessary, since it's only used once?

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -3840,14 +3849,15 @@ void GNUELFDumper::printRelRelaReloc(const 
Relocation &R,
 
 template 
 static void printRelocHeaderFields(formatted_raw_ostream &OS, unsigned SType,
-   const typename ELFT::Ehdr &EHeader) {
+   const typename ELFT::Ehdr &EHeader,
+   uint64_t CrelHdr = 0) {
   bool IsRela = SType == ELF::SHT_RELA || SType == ELF::SHT_ANDROID_RELA;
   if (ELFT::Is64Bits)
 OS << "Offset Info Type   Symbol's 
"
   "Value  Symbol's Name";
   else
 OS << " Offset InfoTypeSym. Value  Symbol's Name";
-  if (IsRela)
+  if (IsRela || (SType == ELF::SHT_CREL && (CrelHdr & CREL_HDR_ADDEND)))

jh7370 wrote:

Could we rename the `IsRela` to `HasAddend` or similar, and then add the CREL 
logic to the initialisation, since `IsRela` isn't used for anything else. 
Alternatively, we could simply inline the variable here.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -3908,7 +3933,8 @@ template  void 
GNUELFDumper::printRelocations() {
 HasRelocSections = true;
 
 std::string EntriesNum = "";
-if (Expected NumOrErr = GetEntriesNum(Sec))
+Expected NumOrErr = GetEntriesNum(Sec);
+if (NumOrErr)

jh7370 wrote:

Why has this changed?

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -392,6 +393,73 @@ ELFFile::decode_relrs(Elf_Relr_Range relrs) const {
   return Relocs;
 }
 
+template 
+Expected
+ELFFile::getCrelHeader(ArrayRef Content) const {
+  DataExtractor Data(Content, isLE(), ELFT::Is64Bits ? 8 : 4);

jh7370 wrote:

Not quite what I had in mind, if I'm honest. Perhaps `sizeof(ELFT::Addr)`?

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -474,9 +480,29 @@ struct Elf_Rel_Impl, true>
 : public Elf_Rel_Impl, false> {
   LLVM_ELF_IMPORT_TYPES(Endianness, true)
   static const bool IsRela = true;
+  static const bool IsCrel = false;
   Elf_Sxword r_addend; // Compute value for relocatable field by adding this.
 };
 
+// In-memory representation. The serialized representation uses LEB128.
+template  struct Elf_Crel_Impl {
+  using uint = std::conditional_t;
+  static const bool IsRela = true;

jh7370 wrote:

This variable feels misnamed now. If its meaning is "relocation contains 
addend" rather than "relocation's addend is in the code" or something (I 
haven't attempted to figure out if that is what this is actually used for), 
then perhaps it needs renaming to something else.

After all, when I read the variable name, I think it means "is this an 
Elf_Rela" relocation, when the answer should be "no" for this type, since it is 
an "Elf_Crel".

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits


@@ -392,6 +393,73 @@ ELFFile::decode_relrs(Elf_Relr_Range relrs) const {
   return Relocs;
 }
 
+template 
+Expected
+ELFFile::getCrelHeader(ArrayRef Content) const {
+  DataExtractor Data(Content, isLE(), ELFT::Is64Bits ? 8 : 4);
+  Error Err = Error::success();
+  uint64_t Hdr = 0;
+  Hdr = Data.getULEB128(&Hdr, &Err);
+  if (Err)
+return Err;
+  return Hdr;
+}
+
+template 
+Expected::RelsOrRelas>
+ELFFile::decodeCrel(ArrayRef Content) const {
+  DataExtractor Data(Content, isLE(), ELFT::Is64Bits ? 8 : 4);
+  DataExtractor::Cursor Cur(0);
+  const uint64_t Hdr = Data.getULEB128(Cur);
+  const size_t Count = Hdr / 8;
+  const size_t FlagBits = Hdr & ELF::CREL_HDR_ADDEND ? 3 : 2;
+  const size_t Shift = Hdr % ELF::CREL_HDR_ADDEND;
+  std::vector Rels;
+  std::vector Relas;
+  if (Hdr & ELF::CREL_HDR_ADDEND)
+Relas.resize(Count);
+  else
+Rels.resize(Count);
+  typename ELFT::uint Offset = 0, Addend = 0;
+  uint32_t Symidx = 0, Type = 0;

jh7370 wrote:

Nit, since it is short for symbol index (i.e. two words). Same below.
```suggestion
  uint32_t SymIdx = 0, Type = 0;
```

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-19 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I've spent as much time as I can on this today. I've reviewed the code in its 
entirety, but still haven't tackled the tests, I'm afraid.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-18 Thread James Henderson via cfe-commits

jh7370 wrote:

> [jh7370](https://github.com/jh7370) sorry for pinging again. I will be highly 
> grateful if you kindly review this pull request. This change will unblock 
> some of our internal tasks and timely completion of this is crucial for us. 
> Thank you in advance.

I hope to look at this in the next day or two - I've had my head down in some 
local work, so have avoided more complex reviews the past week.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-07 Thread James Henderson via cfe-commits


@@ -86,6 +86,8 @@ DYNAMIC_TAG(RELRSZ, 35)  // Size of Relr relocation table.
 DYNAMIC_TAG(RELR, 36)// Address of relocation table (Relr entries).
 DYNAMIC_TAG(RELRENT, 37) // Size of a Relr relocation entry.
 
+DYNAMIC_TAG(CREL,  38)   // CREL relocation table
+

jh7370 wrote:

> I understand the concern but this is just impossible given the essential 
> state of the generic ABI...

I don't understand this comment. There's an active mailing list, where 
proposals are discussed and adopted, even if there's no fully published 
document, so a different proposal could easily come up whilst this is still 
under review. Perhaps it'll be implemented in GNU/Solaris/... initially with 
the same generic value. Whose meaning of the value should then win?

In the worst case, maybe the generic ABI list ends up rejecting this crel 
proposal. The values aren't reserved for LLVM usage, so the next proposal to 
come along will use those same values and we then have a clash between what 
LLVM wants to use the value for (and there might be objects out there using 
that value, if this feature is adopted in an LLVM release), which would cause 
us all sorts of headaches in the future.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-06-04 Thread James Henderson via cfe-commits

jh7370 wrote:

> (Friendly Ping:)

Sorry, a combination of PTO and other factors have delayed me in getting to 
this. It's on my radar, but will likely be a few more days before I can sink my 
teeth into more complex reviews like this.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -934,10 +943,51 @@ void ELFWriter::WriteSecHdrEntry(uint32_t Name, uint32_t 
Type, uint64_t Flags,
   WriteWord(EntrySize); // sh_entsize
 }
 
+template 
+static void encodeCrel(ArrayRef Relocs, raw_ostream &os) {

jh7370 wrote:

Nit: `os` -> `OS`

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -259,7 +260,7 @@ class ELFObjectWriter : public MCObjectWriter {
   void recordRelocation(MCAssembler &Asm, const MCAsmLayout &Layout,
 const MCFragment *Fragment, const MCFixup &Fixup,
 MCValue Target, uint64_t &FixedValue) override;
-  bool usesRela(const MCSectionELF &Sec) const;
+  bool usesRela(const MCTargetOptions *, const MCSectionELF &Sec) const;

jh7370 wrote:

Should the options parameter be named here?

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -86,6 +86,8 @@ DYNAMIC_TAG(RELRSZ, 35)  // Size of Relr relocation table.
 DYNAMIC_TAG(RELR, 36)// Address of relocation table (Relr entries).
 DYNAMIC_TAG(RELRENT, 37) // Size of a Relr relocation entry.
 
+DYNAMIC_TAG(CREL,  38)   // CREL relocation table
+

jh7370 wrote:

I think we need to avoid generic numbers for dynamic tags/section types etc, 
except possibly generic numbers that are a long way from the "normal" range. 
The reason for this is we don't know what values are going to get standardised, 
assuming this feature even gets accepted into the standard. If, for example, 
another feature gets accepted into the standard before CREL, it might use up 
the slot you've "allocated" for the DT_CREL here, which could cause problems 
with loaders that have supported the experimental CREL implementation. On the 
other hand, a different special number (whether in a dedicated range or 
otherwise) that isn't adjacent to the list will not have the issue of a 
potential clash.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -392,6 +393,70 @@ ELFFile::decode_relrs(Elf_Relr_Range relrs) const {
   return Relocs;
 }
 
+template 
+uint64_t ELFFile::crelHeader(ArrayRef Content) const {
+  DataExtractor Data(Content, true, 8); // endian/class is irrelevant

jh7370 wrote:

Endian/class may be irrelevant, but since you have access to the ELFT here 
anyway, would it not make sense to just use the corresponding values? Same 
elsewhere.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -474,9 +480,28 @@ struct Elf_Rel_Impl, true>
 : public Elf_Rel_Impl, false> {
   LLVM_ELF_IMPORT_TYPES(Endianness, true)
   static const bool IsRela = true;
+  static const bool IsCrel = false;
   Elf_Sxword r_addend; // Compute value for relocatable field by adding this.
 };
 
+template  struct Elf_Crel_Impl {
+  using uint = std::conditional_t;

jh7370 wrote:

If we passed the ELFType down the stack rather than just the Is64 boolean, we 
wouldn't need this extra `using`, since there already exists a `uint` defined 
in the ELFType class.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -392,6 +393,70 @@ ELFFile::decode_relrs(Elf_Relr_Range relrs) const {
   return Relocs;
 }
 
+template 
+uint64_t ELFFile::crelHeader(ArrayRef Content) const {
+  DataExtractor Data(Content, true, 8); // endian/class is irrelevant
+  DataExtractor::Cursor Cur(0);
+  uint64_t Hdr = Data.getULEB128(Cur);
+  // In case of an error, return 0 and postpone error reporting to decodeCrel.
+  consumeError(Cur.takeError());
+  return Hdr;
+}
+
+template 
+Expected::RelsOrRelas>
+ELFFile::decodeCrel(ArrayRef Content) const {
+  DataExtractor Data(Content, true, 8); // endian/class is irrelevant
+  DataExtractor::Cursor Cur(0);
+  const uint64_t Hdr = Data.getULEB128(Cur);
+  const size_t Count = Hdr / 8, FlagBits = Hdr & ELF::CREL_HDR_ADDEND ? 3 : 2,
+   Shift = Hdr % ELF::CREL_HDR_ADDEND;

jh7370 wrote:

This line is unnecessarily complicated. It would be much clearer to just define 
each variable on a separate line.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -392,6 +393,70 @@ ELFFile::decode_relrs(Elf_Relr_Range relrs) const {
   return Relocs;
 }
 
+template 
+uint64_t ELFFile::crelHeader(ArrayRef Content) const {
+  DataExtractor Data(Content, true, 8); // endian/class is irrelevant
+  DataExtractor::Cursor Cur(0);
+  uint64_t Hdr = Data.getULEB128(Cur);
+  // In case of an error, return 0 and postpone error reporting to decodeCrel.
+  consumeError(Cur.takeError());

jh7370 wrote:

Throwing away the error like this means you lose the context about what went 
wrong when decoding. That doesn't seem great to me.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -61,6 +61,9 @@ class MCTargetOptions {
 
   bool Dwarf64 : 1;
 
+  // Use CREL relocation format for ELF.
+  bool Crel = false;

jh7370 wrote:

Nit here and elsewhere: Is "Crel" a correct way of spelling this? Should it be 
"CRel" (for "CompressedRelocations")? I'm assuming you're deliberately avoiding 
using CREL in this context, but an argument could be made for that too.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -474,9 +480,28 @@ struct Elf_Rel_Impl, true>
 : public Elf_Rel_Impl, false> {
   LLVM_ELF_IMPORT_TYPES(Endianness, true)
   static const bool IsRela = true;
+  static const bool IsCrel = false;
   Elf_Sxword r_addend; // Compute value for relocatable field by adding this.
 };
 
+template  struct Elf_Crel_Impl {
+  using uint = std::conditional_t;
+  static const bool IsRela = true;
+  static const bool IsCrel = true;
+  uint r_offset;
+  uint32_t r_symidx;
+  uint32_t r_type;
+  std::conditional_t r_addend;

jh7370 wrote:

Again, if I'm not mistaken, using the ELFType would avoid the need for the 
`std::conditional` here: you could use the Elf_Addend type, I believe.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -934,10 +943,51 @@ void ELFWriter::WriteSecHdrEntry(uint32_t Name, uint32_t 
Type, uint64_t Flags,
   WriteWord(EntrySize); // sh_entsize
 }
 
+template 
+static void encodeCrel(ArrayRef Relocs, raw_ostream &os) {
+  uint OffsetMask = 8, Offset = 0, Addend = 0;
+  uint32_t Symidx = 0, Type = 0;
+  // hdr & 4 indicates 3 flag bits in delta offset and flags members.
+  for (size_t i = 0, e = Relocs.size(); i != e; ++i)

jh7370 wrote:

Nit: `i` and `e` -> `I` and `E`.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I've made a start on this, but have run out of time. Will come back to 
reviewing it another day.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits


@@ -321,6 +321,11 @@ class ELFFile {
 
   std::vector decode_relrs(Elf_Relr_Range relrs) const;
 
+  uint64_t crelHeader(ArrayRef Content) const;

jh7370 wrote:

It's not clear without looking at the method definition what the return value 
actually represents. Indeed, this method name doesn't seem to conform to the 
LLVM naming guidelines, since it's neither a verb or adjective phrase or 
similar. Perhaps renaming the method would be wise (and also possibly a comment 
to explain it, if needed still after the renaming).

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-23 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC,llvm-readobj,yaml2obj] Support CREL relocation format (PR #91280)

2024-05-20 Thread James Henderson via cfe-commits

jh7370 wrote:

Just noting that this is on my radar, but I haven't found time to look at it 
properly yet.

https://github.com/llvm/llvm-project/pull/91280
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [compiler-rt] [flang] [libc] [libclc] [libcxx] [libcxxabi] [libunwind] [lld] [lldb] [llvm] [mlir] [openmp] [polly] [pstl] Update IDE Folders (PR #89153)

2024-04-23 Thread James Henderson via cfe-commits

jh7370 wrote:

> This looks like a nice improvement for folks using those generators. Even 
> though most of these changes look straightforward, it would be a lot easier 
> to review if this was broken up per subproject. Is there any reason that's 
> not possible?

+1 to this: 195 files is too many to review in a single PR really, so if we 
have an alternative, incremental approach, we should take that. (Also big +1 to 
the improvement though).

https://github.com/llvm/llvm-project/pull/89153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [llvm-ar][Archive] Use getDefaultTargetTriple instead of host triple for the fallback archive format. (PR #82888)

2024-02-27 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/82888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [llvm-ar][Archive] Use getDefaultTargetTriple instead of host triple for the fallback archive format. (PR #82888)

2024-02-27 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/82888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [llvm-ar][Archive] Use getDefaultTargetTriple instead of host triple for the fallback archive format. (PR #82888)

2024-02-27 Thread James Henderson via cfe-commits

https://github.com/jh7370 approved this pull request.

LGTM. I considered if there was any way of testing this, but the only way I 
ould think of was to have lit know the default target triple (it might well do 
that) and then somehow leverage that to convert to a check for the "expected" 
format for an empty archive. I guess a python script that takes the default 
triple and inspects the output archive might be possible, but it's may not be 
worth it. If you want to go down that route, feel free (and I'd be happy to 
review), but I'm not going to require it.

https://github.com/llvm/llvm-project/pull/82888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [llvm-ar][Archive] Use getDefaultTargetTriple instead of host triple for the fallback archive format. (PR #82888)

2024-02-26 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I think the new behaviour makes more sense to me than the old one, thanks, but 
I think it should be documented both in the release notes and the llvm-ar 
Command Guide document.

https://github.com/llvm/llvm-project/pull/82888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[mlir] [openmp] [libc] [libcxx] [libclc] [lld] [lldb] [clang-tools-extra] [compiler-rt] [clang] [llvm] Make llvm-strip not eat the .gnu_debuglink section (PR #78919)

2024-01-22 Thread James Henderson via cfe-commits

https://github.com/jh7370 requested changes to this pull request.

Hi, thanks for the contribution.

Please could you add a new lit test case to show that this behaviour works as 
intended.

I note that this code as-is only impacts `--strip-all` behaviour. This means 
that if you use `--strip-debug`, the section will get removed. Is this 
intentional? What do GNU strip or objcopy do in these cases?

https://github.com/llvm/llvm-project/pull/78919
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [lld] [clang] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)

2024-01-11 Thread James Henderson via cfe-commits


@@ -1419,32 +1419,50 @@ void ELFState::writeSectionContent(
   CBA.write(E.Feature);
   SHeader.sh_size += 2;
 }
-
-if (Section.PGOAnalyses) {
-  if (E.Version < 2)
-WithColor::warning()
-<< "unsupported SHT_LLVM_BB_ADDR_MAP version when using PGO: "
-<< static_cast(E.Version) << "; must use version >= 2";
+auto FeatureOrErr = llvm::object::BBAddrMap::Features::decode(E.Feature);
+bool MultiBBRangeFeatureEnabled = false;
+if (!FeatureOrErr)
+  WithColor::warning() << toString(FeatureOrErr.takeError());
+else
+  MultiBBRangeFeatureEnabled = FeatureOrErr->MultiBBRange;
+bool MultiBBRange =
+MultiBBRangeFeatureEnabled ||
+(E.NumBBRanges.has_value() && E.NumBBRanges.value() > 1) ||
+(E.BBRanges && E.BBRanges->size() > 1);
+if (MultiBBRange && !MultiBBRangeFeatureEnabled)
+  WithColor::warning() << "Feature value(" << E.Feature

jh7370 wrote:

```suggestion
  WithColor::warning() << "feature value(" << E.Feature
```
Nit, per coding standards.

https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [lld] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)

2024-01-11 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

Sorry, I'm a bit snowed under with reviews. I'm happy to defer to others on 
this patch.

https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[lld] [llvm] [clang] [SHT_LLVM_BB_ADDR_MAP] Allow basic-block-sections and labels be used together by decoupling the handling of the two features. (PR #74128)

2024-01-11 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/74128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang-tools-extra] [clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-12-12 Thread James Henderson via cfe-commits


@@ -234,11 +234,11 @@ defm update_section
 defm gap_fill
 : Eq<"gap-fill", "Fill the gaps between sections with  instead of 
zero. "
  " must be an unsigned 8-bit integer. "
- "This option is only supported for ELF inputs and binary 
outputs.">,
+ "This option is only supported for the ELF input and 
binary output">,
   MetaVarName<"value">;
 
 defm pad_to
-: Eq<"pad-to", "Pad the output to the load address , using a 
value "
-   "of zero or the value specified by :option:`--gap-fill`"
-   "This option is only supported for ELF inputs and binary 
outputs.">,
+: Eq<"pad-to", "Pad the output up to the load address , using a 
value "
+   "of zero or the value specified by the --gap-fill option. "
+   "This option is only supported for the ELF input and binary 
output">,

jh7370 wrote:

Ditto: no "the" before "ELF".

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-12-12 Thread James Henderson via cfe-commits


@@ -234,11 +234,11 @@ defm update_section
 defm gap_fill
 : Eq<"gap-fill", "Fill the gaps between sections with  instead of 
zero. "
  " must be an unsigned 8-bit integer. "
- "This option is only supported for ELF inputs and binary 
outputs.">,
+ "This option is only supported for the ELF input and 
binary output">,

jh7370 wrote:

I don't think you want "the" before "ELF".

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-12-12 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

I think you should undo the `reportWarning` changes: there's no guarantee 
anybody will need it in the future, so you shouldn't change it now.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang-tools-extra] [clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-12-12 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [llvm] [XCOFF][obj2yaml] support parsing auxiliary symbols for XCOFF (PR #70642)

2023-12-05 Thread James Henderson via cfe-commits


@@ -106,6 +126,210 @@ Error XCOFFDumper::dumpSections(ArrayRef Sections) {
   return Error::success();
 }
 
+Error XCOFFDumper::dumpFileAuxSym(XCOFFYAML::Symbol &Sym,
+  const XCOFFSymbolRef &SymbolEntRef) {
+  for (uint8_t I = 1; I <= Sym.NumberOfAuxEntries; ++I) {
+uintptr_t AuxAddress = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
+SymbolEntRef.getEntryAddress(), I);
+const XCOFFFileAuxEnt *FileAuxEntPtr =
+getAuxEntPtr(AuxAddress);
+auto FileNameOrError = Obj.getCFileName(FileAuxEntPtr);
+if (!FileNameOrError)
+  return FileNameOrError.takeError();
+
+XCOFFYAML::FileAuxEnt FileAuxSym;
+FileAuxSym.FileNameOrString = FileNameOrError.get();
+FileAuxSym.FileStringType = FileAuxEntPtr->Type;
+Sym.AuxEntries.push_back(

jh7370 wrote:

Sorry, I'm not really sure I follow the conversation, as it is too much in the 
file format details. Some general comments:
1) We need enough info in the YAML produced by obj2yaml for yaml2obj to be able 
to create an identical object as was passed to obj2yaml.
2) This means we can potentially omit fields from the YAML dump if they have a 
value matching the default value yaml2obj would use if the field is omitted.
3) The less data we have in the YAML, the better, as long as the first point is 
conformed to. This is because it'll be easier to read, and if somebody is using 
obj2yaml to generate the YAML that will be used as the input in a test case, it 
is minimal and has no redundant information.

https://github.com/llvm/llvm-project/pull/70642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm] [clang] [llvm-objcopy] Check for missing argument values (PR #70710)

2023-11-14 Thread James Henderson via cfe-commits

https://github.com/jh7370 approved this pull request.

Are there any options that expect 2+ values (i.e. something that would give a 
different response to "expected 1 value(s)"? If so, it might be worth switching 
one of the cases for that one, just for additional bonus coverage without 
additional test complexity.

Otherwise, LGTM.

https://github.com/llvm/llvm-project/pull/70710
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-11-01 Thread James Henderson via cfe-commits

https://github.com/jh7370 approved this pull request.

I'd like @MaskRay to give this a once-over before merging this, but otherwise 
LGTM.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang-tools-extra] [llvm] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-31 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang-tools-extra] [clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-31 Thread James Henderson via cfe-commits

https://github.com/jh7370 commented:

Basically looks good. Just some minor nits now.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-31 Thread James Henderson via cfe-commits


@@ -67,12 +67,18 @@ using namespace llvm::object;
 // The name this program was invoked as.
 static StringRef ToolName;
 
-static ErrorSuccess reportWarning(Error E) {
+namespace llvm {
+namespace objcopy {
+
+ErrorSuccess reportWarning(Error E) {

jh7370 wrote:

Please follow the guideline described here: 
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [llvm] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-31 Thread James Henderson via cfe-commits


@@ -163,8 +163,8 @@ Sections:
 EntSize: 0x0001
 Content: 4743433A
 
-## In this test, output sections are defined out of in respect to their load
-## addresses. Verify that gaps are still correctly filled.
+## In this test, output sections are defined out of order in respect to their

jh7370 wrote:

```suggestion
## In this test, output sections are defined out of order with respect to their
```
Nit: the phrase is "with respect to".

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang-tools-extra] [clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-31 Thread James Henderson via cfe-commits


@@ -2636,30 +2636,30 @@ template  Error ELFWriter::finalize() 
{
 }
 
 Error BinaryWriter::write() {
-  SmallVector LoadableSections;
+  SmallVector BitsSections;

jh7370 wrote:

Without knowing the code around it, I wouldn't know what `BitsSections` means. 
How about `SectionsToWrite`?

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -738,6 +738,37 @@ objcopy::parseObjcopyOptions(ArrayRef 
RawArgsArr,
   if (auto Arg = InputArgs.getLastArg(OBJCOPY_extract_partition))
 Config.ExtractPartition = Arg->getValue();
 
+  if (const auto *A = InputArgs.getLastArg(OBJCOPY_gap_fill)) {
+if (Config.OutputFormat != FileFormat::Binary)
+  return createStringError(
+  errc::invalid_argument,
+  "'--gap-fill' is only supported for binary output");
+ErrorOr Val = getAsInteger(A->getValue());
+if (!Val)
+  return createStringError(Val.getError(), "--gap-fill: bad number: %s",
+   A->getValue());
+uint8_t ByteVal = Val.get();
+if (ByteVal != Val.get())
+  llvm::errs() << "warning: truncating gap-fill from 0x"
+   << llvm::utohexstr(Val.get(), true) << " to 0x"
+   << llvm::utohexstr(ByteVal, true) << '\n';

jh7370 wrote:

Don't print direct to `llvm::errs`. This won't follow the proper formatting 
style for warnings produced by tools (see what other tools do). 
llvm-objcopy.cpp already has a `reportWarning` method. It seems reasonable to 
declare that in a header file so that you can also use it here.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -0,0 +1,198 @@
+# RUN: yaml2obj --docnum=1 %s -o %t
+
+# RUN: not llvm-objcopy --gap-fill 1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
+# NOT-BINARY: error: '--gap-fill' is only supported for binary output
+
+# RUN: not llvm-objcopy -O binary --gap-fill= %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-FORMAT
+# BAD-FORMAT: error: --gap-fill: bad number:
+
+# RUN: not llvm-objcopy -O binary --gap-fill=x %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT
+# BAD-INPUT: error: --gap-fill: bad number: x
+
+# RUN: not llvm-objcopy -O binary --gap-fill=0x %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT2
+# BAD-INPUT2: error: --gap-fill: bad number: 0x
+
+# RUN: not llvm-objcopy -O binary --gap-fill=0x1G %t %t.bin 2>&1 | FileCheck 
%s --check-prefix=BAD-INPUT3
+# BAD-INPUT3: error: --gap-fill: bad number: 0x1G
+
+# RUN: not llvm-objcopy -O binary --gap-fill=ff %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT4
+# BAD-INPUT4: error: --gap-fill: bad number: ff
+
+# RUN: llvm-objcopy -O binary --gap-fill=0x1122 %t %t-val16 2>&1 | FileCheck 
%s --check-prefix=TRUNCATED-ERR
+# TRUNCATED-ERR: warning: truncating gap-fill from 0x1122 to 0x22
+
+# RUN: od -v -Ax -t x1 %t-val16 | FileCheck %s --check-prefix=TRUNCATED 
--match-full-lines
+# TRUNCATED:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 22 a1 b2
+# TRUNCATED-NEXT: 10 c3 d4 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 20 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 30 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 40 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 50 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 60 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 70 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 80 22 22 89 ab cd ef
+# TRUNCATED-NEXT: 86
+
+## Test no gap fill with all allocatable output sections.
+# RUN: llvm-objcopy -O binary %t %t-default
+# RUN: od -v -Ax -t x1 %t-default | FileCheck %s --check-prefix=DEFAULT 
--match-full-lines
+# DEFAULT:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 00 a1 b2
+# DEFAULT-NEXT: 10 c3 d4 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 50 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 60 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 70 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 80 00 00 89 ab cd ef
+# DEFAULT-NEXT: 86
+
+## Test gap fill with all allocatable output sections.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 %t %t-filled
+# RUN: od -v -Ax -t x1 %t-filled | FileCheck %s --check-prefix=FULL 
--match-full-lines
+# FULL:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba e9 a1 b2
+# FULL-NEXT: 10 c3 d4 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 20 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 30 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 40 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 50 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 60 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 70 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 80 e9 e9 89 ab cd ef
+# FULL-NEXT: 86
+
+## Test gap fill with a decimal value.
+# RUN: llvm-objcopy -O binary --gap-fill=99 %t %t-filled-decimal
+# RUN: od -v -Ax -t x1 %t-filled-decimal | FileCheck %s --check-prefix=DEC 
--match-full-lines
+# DEC:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 63 a1 b2
+# DEC-NEXT: 10 c3 d4 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 30 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 40 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 50 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 60 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 70 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 80 63 63 89 ab cd ef
+# DEC-NEXT: 86
+
+## Test gap fill with the last section removed, should be truncated.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 --remove-section=.foo %t 
%t-filled
+# RUN: od -v -Ax -t x1 %t-filled | FileCheck %s 
--check-prefix=REMOVE-LAST-SECTION --match-full-lines
+# REMOVE-LAST-SECTION: 00 ee ff 11 22 33 44 aa bb cc dd fe dc ba e9 a1 b2
+# REMOVE-LAST-SECTION-NEXT: 10 c3 d4
+# REMOVE-LAST-SECTION-NEXT: 12
+
+## Test gap fill with the middle section removed, should be filled.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 --remove-section=.gap2 %t 
%t-filled
+# RUN: od 

[clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -0,0 +1,94 @@
+# RUN: yaml2obj %s -o %t
+
+# RUN: not llvm-objcopy --pad-to=1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
+# NOT-BINARY: error: '--pad-to' is only supported for binary output
+
+# RUN: not llvm-objcopy -O binary --pad-to= %t 2>&1 | FileCheck %s 
--check-prefix=BAD-FORMAT
+# BAD-FORMAT: error: --pad-to: bad number:
+
+# RUN: not llvm-objcopy -O binary --pad-to=x %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT
+# BAD-INPUT: error: --pad-to: bad number: x
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x1G %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT2
+# BAD-INPUT2: error: --pad-to: bad number: 0x1G
+
+# RUN: not llvm-objcopy -O binary --pad-to=ff %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT3
+# BAD-INPUT3: error: --pad-to: bad number: ff
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x112233445566778899 %t 2>&1 | 
FileCheck %s --check-prefix=BAD-NUMBER
+# BAD-NUMBER: error: --pad-to: bad number: 0x112233445566778899
+
+## Save the baseline, not padded output.
+# RUN: llvm-objcopy -O binary %t %t.bin
+
+## Pad to an address smaller than the binary size.
+# RUN: llvm-objcopy -O binary --pad-to=0x20 %t %t-p1
+# RUN: cmp %t.bin %t-p1
+# RUN: llvm-objcopy -O binary --pad-to=0x200 %t %t-p2
+# RUN: cmp %t.bin %t-p2
+
+## Pad all allocatable sections to a valid address.
+# RUN: llvm-objcopy -O binary --pad-to=0x218 %t %t-pad-default
+# RUN: od -v -Ax -t x1 %t-pad-default | FileCheck %s --check-prefix=DEFAULT 
--match-full-lines
+# DEFAULT:  00 11 22 33 44 55 66 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 10 77 88 99 aa 00 00 00 00
+# DEFAULT-NEXT: 18
+
+## Use a decimal number of padding address, verify it's not misunderstood.

jh7370 wrote:

```suggestion
## Use a decimal number for the padding address and verify it's not 
misunderstood.
```

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -1,39 +1,32 @@
-# RUN: yaml2obj %s > %t
+# RUN: yaml2obj %s -o %t
 
-## Verify section headers before we perform several testings.
-# RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=ORG-SHDR
-# ORG-SHDR: Section Headers:
-# ORG-SHDR:  [Nr] Name  TypeAddress  Off
Size   ES Flg Lk Inf Al
-# ORG-SHDR:  [ 0]   NULL 00 
00 00  0   0  0
-# ORG-SHDR:  [ 1] .nogapPROGBITS0102 42 
06 00   A  0   0  1
-# ORG-SHDR:  [ 2] .gap1 PROGBITS0108 48 
07 00  AX  0   0  1
-# ORG-SHDR:  [ 3] .gap2 PROGBITS0110 50 
04 00   A  0   0  1
-# ORG-SHDR:  [ 4] .nobit_tbss   NOBITS  0180 58 
18 00 WAT  0   0  8
-# ORG-SHDR:  [ 5] .foo  PROGBITS0184 5c 
04 00  WA  0   0  1
-# ORG-SHDR:  [ 6] .nobit_bssNOBITS  018a 60 
08 00  WA  0   0  1
+## This test is partially based on one from D67689.
 
 # RUN: not llvm-objcopy --gap-fill 1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
 # NOT-BINARY: error: '--gap-fill' is only supported for binary output
 
 # RUN: not llvm-objcopy -O binary --gap-fill %t 2>&1 | FileCheck %s 
--check-prefix=EMPTY

jh7370 wrote:

> if the last option in the command line has no value, it is silently ignored.

This was the case that surprised me. I feel like this is a bug in the 
command-line option parser (it's more generic than for llvm-objcopy). Could you 
file a bug for this, please?

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -738,6 +738,37 @@ objcopy::parseObjcopyOptions(ArrayRef 
RawArgsArr,
   if (auto Arg = InputArgs.getLastArg(OBJCOPY_extract_partition))
 Config.ExtractPartition = Arg->getValue();
 
+  if (const auto *A = InputArgs.getLastArg(OBJCOPY_gap_fill)) {
+if (Config.OutputFormat != FileFormat::Binary)
+  return createStringError(
+  errc::invalid_argument,
+  "'--gap-fill' is only supported for binary output");
+ErrorOr Val = getAsInteger(A->getValue());
+if (!Val)
+  return createStringError(Val.getError(), "--gap-fill: bad number: %s",
+   A->getValue());
+uint8_t ByteVal = Val.get();
+if (ByteVal != Val.get())
+  llvm::errs() << "warning: truncating gap-fill from 0x"
+   << llvm::utohexstr(Val.get(), true) << " to 0x"
+   << llvm::utohexstr(ByteVal, true) << '\n';
+Config.GapFill = ByteVal;
+  } else
+Config.GapFill = 0; // The value of zero is equivalent to no fill.

jh7370 wrote:

The value of `GapFill` is initialized to 0 already. Do you really need this? 
Same below for `PadTo`.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -2635,9 +2635,36 @@ template  Error ELFWriter::finalize() {
 }
 
 Error BinaryWriter::write() {
-  for (const SectionBase &Sec : Obj.allocSections())
+  SmallVector LoadableSections;
+  for (const SectionBase &Sec : Obj.allocSections()) {
+if (Sec.Type != SHT_NOBITS)
+  LoadableSections.push_back(&Sec);

jh7370 wrote:

NOBITS sections are loadable, so I don't think this variable name makes sense.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -0,0 +1,94 @@
+# RUN: yaml2obj %s -o %t
+
+# RUN: not llvm-objcopy --pad-to=1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
+# NOT-BINARY: error: '--pad-to' is only supported for binary output
+
+# RUN: not llvm-objcopy -O binary --pad-to= %t 2>&1 | FileCheck %s 
--check-prefix=BAD-FORMAT
+# BAD-FORMAT: error: --pad-to: bad number:
+
+# RUN: not llvm-objcopy -O binary --pad-to=x %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT
+# BAD-INPUT: error: --pad-to: bad number: x
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x1G %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT2
+# BAD-INPUT2: error: --pad-to: bad number: 0x1G
+
+# RUN: not llvm-objcopy -O binary --pad-to=ff %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT3
+# BAD-INPUT3: error: --pad-to: bad number: ff
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x112233445566778899 %t 2>&1 | 
FileCheck %s --check-prefix=BAD-NUMBER
+# BAD-NUMBER: error: --pad-to: bad number: 0x112233445566778899
+
+## Save the baseline, not padded output.
+# RUN: llvm-objcopy -O binary %t %t.bin
+
+## Pad to an address smaller than the binary size.
+# RUN: llvm-objcopy -O binary --pad-to=0x20 %t %t-p1
+# RUN: cmp %t.bin %t-p1
+# RUN: llvm-objcopy -O binary --pad-to=0x200 %t %t-p2
+# RUN: cmp %t.bin %t-p2
+
+## Pad all allocatable sections to a valid address.
+# RUN: llvm-objcopy -O binary --pad-to=0x218 %t %t-pad-default
+# RUN: od -v -Ax -t x1 %t-pad-default | FileCheck %s --check-prefix=DEFAULT 
--match-full-lines
+# DEFAULT:  00 11 22 33 44 55 66 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 10 77 88 99 aa 00 00 00 00
+# DEFAULT-NEXT: 18
+
+## Use a decimal number of padding address, verify it's not misunderstood.

jh7370 wrote:

```suggestion
## Use a decimal number for the padding address and verify it's not 
misunderstood.
```

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -0,0 +1,94 @@
+# RUN: yaml2obj %s -o %t
+
+# RUN: not llvm-objcopy --pad-to=1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
+# NOT-BINARY: error: '--pad-to' is only supported for binary output
+
+# RUN: not llvm-objcopy -O binary --pad-to= %t 2>&1 | FileCheck %s 
--check-prefix=BAD-FORMAT
+# BAD-FORMAT: error: --pad-to: bad number:
+
+# RUN: not llvm-objcopy -O binary --pad-to=x %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT
+# BAD-INPUT: error: --pad-to: bad number: x
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x1G %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT2
+# BAD-INPUT2: error: --pad-to: bad number: 0x1G
+
+# RUN: not llvm-objcopy -O binary --pad-to=ff %t 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT3
+# BAD-INPUT3: error: --pad-to: bad number: ff
+
+# RUN: not llvm-objcopy -O binary --pad-to=0x112233445566778899 %t 2>&1 | 
FileCheck %s --check-prefix=BAD-NUMBER
+# BAD-NUMBER: error: --pad-to: bad number: 0x112233445566778899
+
+## Save the baseline, not padded output.
+# RUN: llvm-objcopy -O binary %t %t.bin
+
+## Pad to an address smaller than the binary size.
+# RUN: llvm-objcopy -O binary --pad-to=0x20 %t %t-p1
+# RUN: cmp %t.bin %t-p1
+# RUN: llvm-objcopy -O binary --pad-to=0x200 %t %t-p2
+# RUN: cmp %t.bin %t-p2
+
+## Pad all allocatable sections to a valid address.
+# RUN: llvm-objcopy -O binary --pad-to=0x218 %t %t-pad-default
+# RUN: od -v -Ax -t x1 %t-pad-default | FileCheck %s --check-prefix=DEFAULT 
--match-full-lines
+# DEFAULT:  00 11 22 33 44 55 66 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 10 77 88 99 aa 00 00 00 00
+# DEFAULT-NEXT: 18
+
+## Use a decimal number of padding address, verify it's not misunderstood.
+# RUN: llvm-objcopy -O binary --pad-to=536 %t %t-pad-decimal
+# RUN: od -v -Ax -t x1 %t-pad-decimal | FileCheck %s --check-prefix=DECIMAL 
--match-full-lines
+# DECIMAL:  00 11 22 33 44 55 66 00 00 00 00 00 00 00 00 00 00
+# DECIMAL-NEXT: 10 77 88 99 aa 00 00 00 00
+# DECIMAL-NEXT: 18
+
+## Pad all allocatable sections to a valid address, using --gap-fill.
+# RUN: llvm-objcopy -O binary --pad-to=0x218 --gap-fill=0xe9 %t %t-pad-fill
+# RUN: od -v -Ax -t x1 %t-pad-fill | FileCheck %s --check-prefix=FILL 
--match-full-lines
+# FILL:  00 11 22 33 44 55 66 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FILL-NEXT: 10 77 88 99 aa e9 e9 e9 e9
+# FILL-NEXT: 18
+
+## Test gap fill with a section removed.

jh7370 wrote:

This test file is about `--pad-to`. This comment probably needs expanding to 
better explain the test case.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -0,0 +1,198 @@
+# RUN: yaml2obj --docnum=1 %s -o %t
+
+# RUN: not llvm-objcopy --gap-fill 1 %t 2>&1 | FileCheck %s 
--check-prefix=NOT-BINARY
+# NOT-BINARY: error: '--gap-fill' is only supported for binary output
+
+# RUN: not llvm-objcopy -O binary --gap-fill= %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-FORMAT
+# BAD-FORMAT: error: --gap-fill: bad number:
+
+# RUN: not llvm-objcopy -O binary --gap-fill=x %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT
+# BAD-INPUT: error: --gap-fill: bad number: x
+
+# RUN: not llvm-objcopy -O binary --gap-fill=0x %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT2
+# BAD-INPUT2: error: --gap-fill: bad number: 0x
+
+# RUN: not llvm-objcopy -O binary --gap-fill=0x1G %t %t.bin 2>&1 | FileCheck 
%s --check-prefix=BAD-INPUT3
+# BAD-INPUT3: error: --gap-fill: bad number: 0x1G
+
+# RUN: not llvm-objcopy -O binary --gap-fill=ff %t %t.bin 2>&1 | FileCheck %s 
--check-prefix=BAD-INPUT4
+# BAD-INPUT4: error: --gap-fill: bad number: ff
+
+# RUN: llvm-objcopy -O binary --gap-fill=0x1122 %t %t-val16 2>&1 | FileCheck 
%s --check-prefix=TRUNCATED-ERR
+# TRUNCATED-ERR: warning: truncating gap-fill from 0x1122 to 0x22
+
+# RUN: od -v -Ax -t x1 %t-val16 | FileCheck %s --check-prefix=TRUNCATED 
--match-full-lines
+# TRUNCATED:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 22 a1 b2
+# TRUNCATED-NEXT: 10 c3 d4 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 20 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 30 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 40 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 50 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 60 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 70 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22
+# TRUNCATED-NEXT: 80 22 22 89 ab cd ef
+# TRUNCATED-NEXT: 86
+
+## Test no gap fill with all allocatable output sections.
+# RUN: llvm-objcopy -O binary %t %t-default
+# RUN: od -v -Ax -t x1 %t-default | FileCheck %s --check-prefix=DEFAULT 
--match-full-lines
+# DEFAULT:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 00 a1 b2
+# DEFAULT-NEXT: 10 c3 d4 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 50 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 60 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 70 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+# DEFAULT-NEXT: 80 00 00 89 ab cd ef
+# DEFAULT-NEXT: 86
+
+## Test gap fill with all allocatable output sections.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 %t %t-filled
+# RUN: od -v -Ax -t x1 %t-filled | FileCheck %s --check-prefix=FULL 
--match-full-lines
+# FULL:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba e9 a1 b2
+# FULL-NEXT: 10 c3 d4 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 20 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 30 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 40 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 50 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 60 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 70 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9 e9
+# FULL-NEXT: 80 e9 e9 89 ab cd ef
+# FULL-NEXT: 86
+
+## Test gap fill with a decimal value.
+# RUN: llvm-objcopy -O binary --gap-fill=99 %t %t-filled-decimal
+# RUN: od -v -Ax -t x1 %t-filled-decimal | FileCheck %s --check-prefix=DEC 
--match-full-lines
+# DEC:  00 ee ff 11 22 33 44 aa bb cc dd fe dc ba 63 a1 b2
+# DEC-NEXT: 10 c3 d4 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 30 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 40 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 50 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 60 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 70 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63
+# DEC-NEXT: 80 63 63 89 ab cd ef
+# DEC-NEXT: 86
+
+## Test gap fill with the last section removed, should be truncated.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 --remove-section=.foo %t 
%t-filled
+# RUN: od -v -Ax -t x1 %t-filled | FileCheck %s 
--check-prefix=REMOVE-LAST-SECTION --match-full-lines
+# REMOVE-LAST-SECTION: 00 ee ff 11 22 33 44 aa bb cc dd fe dc ba e9 a1 b2
+# REMOVE-LAST-SECTION-NEXT: 10 c3 d4
+# REMOVE-LAST-SECTION-NEXT: 12
+
+## Test gap fill with the middle section removed, should be filled.
+# RUN: llvm-objcopy -O binary --gap-fill=0xe9 --remove-section=.gap2 %t 
%t-filled
+# RUN: od 

[clang] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)

2023-10-25 Thread James Henderson via cfe-commits


@@ -738,6 +738,37 @@ objcopy::parseObjcopyOptions(ArrayRef 
RawArgsArr,
   if (auto Arg = InputArgs.getLastArg(OBJCOPY_extract_partition))
 Config.ExtractPartition = Arg->getValue();
 
+  if (const auto *A = InputArgs.getLastArg(OBJCOPY_gap_fill)) {
+if (Config.OutputFormat != FileFormat::Binary)
+  return createStringError(
+  errc::invalid_argument,
+  "'--gap-fill' is only supported for binary output");
+ErrorOr Val = getAsInteger(A->getValue());
+if (!Val)
+  return createStringError(Val.getError(), "--gap-fill: bad number: %s",
+   A->getValue());
+uint8_t ByteVal = Val.get();
+if (ByteVal != Val.get())
+  llvm::errs() << "warning: truncating gap-fill from 0x"
+   << llvm::utohexstr(Val.get(), true) << " to 0x"
+   << llvm::utohexstr(ByteVal, true) << '\n';

jh7370 wrote:

Don't print direct to `llvm::errs`. This won't follow the proper formatting 
style for warnings produced by tools (see what other tools do). 
llvm-objcopy.cpp already has a `reportWarning` method. It seems reasonable to 
declare that in a header file so that you can also use it here.

https://github.com/llvm/llvm-project/pull/65815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-10-22 Thread James Henderson via cfe-commits

jh7370 wrote:

I can merge it for you, but first I just want to confirm that all 4 commits in 
this PR are intended to be for the PR? If not, you'll need to rebase and force 
push.

There's no need for you to manually squash the fixups (although you might as 
well if you do end up rebasing). When I do the merge via the UI, it'll be using 
the Squash and merge button, which squashes everything into a single commit.

The final commit message will be the PR description. I found the description a 
little confusing at first, without reading the code and test, so it may be 
beneficial to add an example of the before and after behaviour to that 
description before I merge it. Please could you update the patch description 
accordingly.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-10-19 Thread James Henderson via cfe-commits

jh7370 wrote:

@dankm, is there a particular reason you haven't merged this change in yet?

FWIW, the formatter check failed for some reason, but I'm not sure it's related 
to any formatting issue. Please verify by running clang-format on your changes 
before merging.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-19 Thread James Henderson via cfe-commits


@@ -5,11 +5,14 @@
 # RUN: mkdir %t
 # RUN: ln -s llvm-ranlib %t/llvm-ranlib-9
 # RUN: ln -s llvm-ranlib %t/ranlib.exe
+# RUN: ln -s llvm-ranlib %t/x86_64-unknown-freebsd13.2-llvm-ranlib

jh7370 wrote:

Let's put the new test files and deletion of this old test in a different PR. 
The old code was untested, so we're not making things worse, but it also helps 
keep the PRs focused. Aside: if we're deleting this old file, I think it would 
be a good idea to add one or two cases to the llvm-ar test showing the 
"llvm-ranlib" name.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-19 Thread James Henderson via cfe-commits


@@ -1696,6 +1696,40 @@ TEST(Support, ReplacePathPrefix) {
   EXPECT_EQ(Path, "C:\\old/foo\\bar");
 }
 
+TEST(Support, FindProgramName) {
+  StringRef WindowsProgName =
+  path::program_name("C:\\Test\\foo.exe", path::Style::windows);
+  EXPECT_EQ(WindowsProgName, "foo");
+
+  StringRef WindowsProgNameManyDots = path::program_name(
+  "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows);
+  EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang");
+
+  StringRef PosixProgName =
+  path::program_name("/var/empty/clang.exe", path::Style::posix);
+  EXPECT_EQ(PosixProgName, "clang");
+
+  StringRef PosixProgNameManyDotsExe = path::program_name(
+  "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe",
+  path::Style::posix);
+  EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar");
+
+  StringRef PosixProgNameManyDots = path::program_name(
+  "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix);
+  EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar");
+
+  StringRef PosixProgNameSh =
+  
path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh",
+ path::Style::posix);
+  EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh");
+
+  // TODO: determine if this is correct. What happens on windows with an 
executable
+  // named ".exe"?

jh7370 wrote:

I just used the Windows UI to rename a file to ".exe" and it works fine (both 
renaming and susbequent execution of the file from the command-line). 
Meanwhile, the documented behaviour of `stem` is that something containing only 
a single dot will result in an empty string, which I think we should be 
mirroring here. I think the behaviour tested here is therefore correct and you 
can probably drop the TODO note.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-19 Thread James Henderson via cfe-commits

https://github.com/jh7370 edited https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-18 Thread James Henderson via cfe-commits


@@ -1696,6 +1696,38 @@ TEST(Support, ReplacePathPrefix) {
   EXPECT_EQ(Path, "C:\\old/foo\\bar");
 }
 
+TEST(Support, FindProgramName) {
+  StringRef WindowsProgName =
+  path::program_name("C:\\Test\\foo.exe", path::Style::windows);
+  EXPECT_EQ(WindowsProgName, "foo");
+
+  StringRef WindowsProgNameManyDots = path::program_name(
+  "C:\\Test.7\\x86_64-freebsd14.0-clang.exe", path::Style::windows);
+  EXPECT_EQ(WindowsProgNameManyDots, "x86_64-freebsd14.0-clang");
+
+  StringRef PosixProgName =
+  path::program_name("/var/empty/clang.exe", path::Style::posix);
+  EXPECT_EQ(PosixProgName, "clang");
+
+  StringRef PosixProgNameManyDotsExe = path::program_name(
+  "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.exe",
+  path::Style::posix);
+  EXPECT_EQ(PosixProgNameManyDotsExe, "x86_64-portbld-freebsd13.2-llvm-ar");
+
+  StringRef PosixProgNameManyDots = path::program_name(
+  "/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar", path::Style::posix);
+  EXPECT_EQ(PosixProgNameManyDots, "x86_64-portbld-freebsd13.2-llvm-ar");
+
+  StringRef PosixProgNameSh =
+  
path::program_name("/llvm-test16.4/x86_64-portbld-freebsd13.2-llvm-ar.sh",
+ path::Style::posix);
+  EXPECT_EQ(PosixProgNameSh, "x86_64-portbld-freebsd13.2-llvm-ar.sh");
+
+  StringRef WorseThanFailure =

jh7370 wrote:

`WorseThanFailure` doesn't describe the case particularly well, especially as 
the behaviour here is the same as `stem`. Could it simply be `OnlyExe`?

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-18 Thread James Henderson via cfe-commits


@@ -600,6 +600,14 @@ StringRef extension(StringRef path, Style style) {
   return fname.substr(pos);
 }
 
+StringRef program_name(StringRef path, Style style) {
+  // In the future this may need to be extended to other
+  // program suffixes.

jh7370 wrote:

Unnecessarily wrapped.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-18 Thread James Henderson via cfe-commits


@@ -390,6 +390,21 @@ StringRef stem(StringRef path, Style style = 
Style::native);
 /// @result The extension of \a path.
 StringRef extension(StringRef path, Style style = Style::native);
 
+/// Get the program's name
+///
+/// If the path ends with the string .exe, returns the stem of
+/// \a path. Otherwise returns the filename of \a path.
+///
+/// @code
+///   /foo/prog.exe => prog
+///   /bar/prog => prog
+///   /foo/prog1.2  => prog1.2
+/// @endcode
+///
+/// @param path Input path.
+/// @result The filename of \a path. Without any .exe component

jh7370 wrote:

```suggestion
/// @result The filename of \a path without any ".exe" component.
```

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] Support target names with dots in more utilities (PR #65812)

2023-09-18 Thread James Henderson via cfe-commits


@@ -5,16 +5,22 @@
 # RUN: mkdir %t
 # RUN: ln -s llvm-ar %t/llvm-ar-9
 # RUN: ln -s llvm-ar %t/ar.exe
+# RUN: ln -s llvm-ar %t/x86_64-portbld-freebsd13.2-llvm-ar
+# RUN: ln -s llvm-ar %t/x86_64-portbld-freebsd13.2-llvm-ar.exe
 # RUN: ln -s llvm-ar %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9
 
 # RUN: llvm-ar h | FileCheck %s --check-prefix=DEFAULT
 # RUN: %t/llvm-ar-9 h | FileCheck %s --check-prefix=VERSION
 # RUN: %t/ar.exe h | FileCheck %s --check-prefix=SUFFIX
+# RUN: %t/x86_64-portbld-freebsd13.2-llvm-ar h | FileCheck %s 
--check-prefix=TARGETDOT
+# RUN: %t/x86_64-portbld-freebsd13.2-llvm-ar.exe h | FileCheck %s 
--check-prefix=MULTIDOT
 ## Ensure that the "lib" substring does not result in misidentification as the
 ## llvm-lib tool.
 # RUN: %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9 h | FileCheck %s 
--check-prefix=ARM
 
 # DEFAULT: USAGE: llvm-ar{{ }}
 # VERSION: USAGE: llvm-ar-9{{ }}
 # SUFFIX: USAGE: ar{{ }}
+# TARGETDOT: USAGE: x86_64-portbld-freebsd13.2-llvm-ar{{ }}
+# MULTIDOT: USAGE: x86_64-portbld-freebsd13.2-llvm-ar{{ }}

jh7370 wrote:

These don't need to be separate CHECK patterns, as they check the same thing. 
Simply reuse the prefix and only have one of them.

https://github.com/llvm/llvm-project/pull/65812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [llvm-dev] Phabricator Creator Pulling the Plug

2021-10-01 Thread James Henderson via cfe-commits
+1 to the review experience in Github being far worse than Phabricator,
with basically all my specific concerns already being covered in this
thread. I just wanted to add that our downstream LLVM port is based in a
local Github Enterprise instance, and I find it far harder to review and
respond to reviews there, compared to Phabricator. I'm not just opposed to
change because I fear something new - I have active day-to-day experience
with the something new, based on several years of experience, and I don't
like it! I do acknowledge however, that some things have improved (e.g.
multi-line commenting is now a thing, when it didn't used to be), so it's
not an "absolutely never" from me, if the issues can be solved.

James

On Fri, 1 Oct 2021 at 04:11, Mehdi AMINI via llvm-dev <
llvm-...@lists.llvm.org> wrote:

> On Thu, Sep 30, 2021 at 8:05 PM Hubert Tong
>  wrote:
> >
> > On Thu, Sep 30, 2021 at 6:56 PM Mehdi AMINI via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >>
> >> We talked about this with the IWG (Infrastructure Working Group) just
> >> last week coincidentally.
> >> Two major blocking tracks that were identified at the roundtable
> >> during the LLVM Dev Meeting exactly 2 years ago are still an issue
> >> today:
> >>
> >> 1) Replacement for Herald rules. This is what allows us to subscribe
> >> and track new revisions or commits based on paths in the repo or other
> >> criteria. We could build a replacement based on GitHub action or any
> >> other kind of service, but this is a bit tricky (how do you store
> >> emails privately? etc.). I have looked around online but I didn't find
> >> another OSS project (or external company) providing a similar service
> >> for GitHub unfortunately, does anyone know of any?
> >>
> >> 2) Support for stacked commits. I can see how to structure this
> >> somehow assuming we would push pull-request branches in the main repo
> >> (with one new commit per branch and cascading the pull-requests from
> >> one branch to the other), otherwise this will be a major regression
> >> compared to the current workflow.
> >>
> >> What remains unknown to me is the current state of GitHub management
> >> of comments across `git commit --amend` and force push to update a
> >> branch.
> >
> >
> > Force pushing to a PR branch does make it harder for reviewers to see
> how review comments were addressed or what was done since they last looked
> at the PR. Are your use cases addressed if the workflow consists of pushing
> additional commits to address comments or pushing a merge commit to refresh
> the PR branch? When the PR is approved, the "squash and merge" option can
> be used to commit the patch as a single commit.
>
> This isn't compatible with stacked commits / stacked PR unfortunately.
> Also while merging main back into a branch of commits is "OK",
> rebasing multiple commits is much less friendly (the same conflict may
> have to be fixed over and over in each commit).
>
> > I find the code review experience in GitHub to be a productivity drain
> compared to Phabricator.
> >
> > Older inline comments are much harder to find in GitHub.
> > Much more clicking needed in GitHub to actually load everything (blocks
> of comments folded away, comments collapsed not because you want them
> collapsed but because someone else or maybe just GitHub thought it should
> be collapsed, source files not loaded).
> > GitHub does not allow inline comments further away than a few lines from
> a change.
>
> Thanks! I have the same feeling, but I didn't have anything specific
> to point to and figured that this is in the scope of "I'll get used to
> it", but you mention some good points here.
>
> Best,
>
> --
> Mehdi
> ___
> LLVM Developers mailing list
> llvm-...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] e406cca - Revert "build: reduce CMake handling for zlib"

2020-01-02 Thread James Henderson via cfe-commits

Author: James Henderson
Date: 2020-01-02T16:02:10Z
New Revision: e406cca5f9a6477c9861717f81c156aa83feeaca

URL: 
https://github.com/llvm/llvm-project/commit/e406cca5f9a6477c9861717f81c156aa83feeaca
DIFF: 
https://github.com/llvm/llvm-project/commit/e406cca5f9a6477c9861717f81c156aa83feeaca.diff

LOG: Revert "build: reduce CMake handling for zlib"

This reverts commit 68a235d07f9e7049c7eb0c8091f37e385327ac28.

This commit broke the clang-x64-windows-msvc build bot and a follow-up
commit did not fix it. Reverting to fix the bot.

Added: 


Modified: 
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
compiler-rt/test/lit.common.configured.in
lld/test/CMakeLists.txt
lld/test/lit.site.cfg.py.in
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/cmake/config-ix.cmake
llvm/include/llvm/Config/config.h.cmake
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/CRC.cpp
llvm/lib/Support/Compression.cpp
llvm/test/CMakeLists.txt
llvm/test/lit.site.cfg.py.in
llvm/unittests/Support/CompressionTest.cpp

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 6f6121f06e56..16e487f07b78 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,13 +9,22 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+if(CLANG_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index e9b35ac01771..520afab6af82 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 0fb51741783e..b4862f74cdd0 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -50,7 +50,7 @@ if config.enable_per_target_runtime_dir:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
+set_default("have_zlib", "@HAVE_LIBZ@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index dc8cedf2ea09..8be42c46dd8a 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,8 +4,17 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
+if(LLD_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_LIBXML2_ENABLED
   )
 

diff  --git a/lld/test/lit.site.cfg.py.in b/lld/test/lit.site.cfg.py.in
index 531fce15839d..02840f8d6a30 100644
--- a/lld/test/lit.site.cfg.py.in
+++ b/lld/test/lit.site.cfg.py.in
@@ -14,7 +14,7 @@ config.lld_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
 config.lld_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.sizeof_void_p = @CMAKE_SIZEOF_VOID_P@
 
 # Support substitution of the tools and libs dirs with user parameters. This is

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
index 7cea013eea7f..0a98f6a15d75 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCom

r371497 - Revert Remove REQUIRES:shell from tests that pass for me on Windows

2019-09-11 Thread James Henderson via cfe-commits
Author: jhenderson
Date: Tue Sep 10 01:48:33 2019
New Revision: 371497

URL: http://llvm.org/viewvc/llvm-project?rev=371497&view=rev
Log:
Revert Remove REQUIRES:shell from tests that pass for me on Windows

This reverts r371478 (git commit a9980f60ce083fa6d5fd03c12c58ca0b293e3d60)

Modified:
cfe/trunk/test/Analysis/crash-trace.c
cfe/trunk/test/CodeGen/thinlto_backend.ll
cfe/trunk/test/Driver/check-time-trace-sections.cpp
cfe/trunk/test/Driver/check-time-trace.cpp
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/test/Driver/crash-report-crashfile.m
cfe/trunk/test/Driver/rewrite-map-in-diagnostics.c
cfe/trunk/test/Format/style-on-command-line.cpp
cfe/trunk/test/Frontend/dependency-gen-has-include.c
cfe/trunk/test/Index/crash-recovery-modules.m
cfe/trunk/test/Modules/at-import-in-framework-header.m
cfe/trunk/test/Modules/builtins.m
cfe/trunk/test/Modules/dependency-dump-dependent-module.m
cfe/trunk/test/Modules/dependency-dump.m
cfe/trunk/test/Modules/implicit-invalidate-common.c
cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/task_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_private_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/taskloop_simd_private_codegen.cpp
cfe/trunk/test/PCH/modified-header-error.c
cfe/trunk/test/Parser/crash-report.c

Modified: cfe/trunk/test/Analysis/crash-trace.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/crash-trace.c?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/Analysis/crash-trace.c (original)
+++ cfe/trunk/test/Analysis/crash-trace.c Tue Sep 10 01:48:33 2019
@@ -1,8 +1,9 @@
 // RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
%s 2>&1 | FileCheck %s
 // REQUIRES: crash-recovery
 
-// Stack traces require back traces.
-// REQUIRES: backtrace
+// FIXME: CHECKs might be incompatible to win32.
+// Stack traces also require back traces.
+// REQUIRES: shell, backtrace
 
 void clang_analyzer_crash(void);
 
@@ -17,6 +18,6 @@ void test() {
 // CHECK: 0.   Program arguments: {{.*}}clang
 // CHECK-NEXT: 1.   parser at end of file
 // CHECK-NEXT: 2. While analyzing stack: 
-// CHECK-NEXT:  #0 Calling inlined at line 14
+// CHECK-NEXT:  #0 Calling inlined at line 15
 // CHECK-NEXT:  #1 Calling test
 // CHECK-NEXT: 3.  {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating 
statement

Modified: cfe/trunk/test/CodeGen/thinlto_backend.ll
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto_backend.ll?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/CodeGen/thinlto_backend.ll (original)
+++ cfe/trunk/test/CodeGen/thinlto_backend.ll Tue Sep 10 01:48:33 2019
@@ -1,4 +1,5 @@
-; REQUIRES: x86-registered-target
+; shell required since the windows bot does not like the "(cd ..."
+; REQUIRES: x86-registered-target, shell
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto_backend.ll
@@ -31,14 +32,10 @@
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-dis %t1.s.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT 
%s
 ; RUN: mkdir -p %T/dir1
-; RUN: cd %T/dir1
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps=cwd
-; RUN: cd ../..
+; RUN: (cd %T/dir1 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd)
 ; RUN: llvm-dis %T/dir1/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; RUN: mkdir -p %T/dir2
-; RUN: cd %T/dir2
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c 
-fthinlto-index=%t.thinlto.bc -save-temps
-; RUN: cd ../..
+; RUN: (cd %T/dir2 && %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x 
ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps)
 ; RUN: llvm-dis %T/dir2/*1.s.3.import.bc -o - | FileCheck 
--check-prefix=CHECK-IMPORT %s
 ; CHECK-IMPORT: define available_externally void @f2()
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s

Modified: cfe/trunk/test/Driver/check-time-trace-sections.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/check-time-trace-sections.cpp?rev=371497&r1=371496&r2=371497&view=diff
==
--- cfe/trunk/test/Driver/check-time-trace-sections.cpp (original)
+++ cfe/trunk/test/Driver/check-time-trace-sections.cpp Tue Sep 10 01:48:33 2019
@@ -1,3 +1,4