[clang] [lld] [llvm] Deprecate the `-fbasic-block-sections=labels` option. (PR #107494)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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
+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"
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
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