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
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
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
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://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
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,
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
@@ -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
+
@@ -4888,6 +4920,34 @@ void ELFDumper::printRelocationsHelper(const
Elf_Shdr ) {
template void ELFDumper::printDynamicRelocationsHelper() {
const bool IsMips64EL = this->Obj.isMips64EL();
+ auto DumpCrelRegion = [&](DynRegionInfo ) {
+// While the size is unknown, a
@@ -4888,6 +4920,34 @@ void ELFDumper::printRelocationsHelper(const
Elf_Shdr ) {
template void ELFDumper::printDynamicRelocationsHelper() {
const bool IsMips64EL = this->Obj.isMips64EL();
+ auto DumpCrelRegion = [&](DynRegionInfo ) {
jh7370 wrote:
The
@@ -3840,14 +3849,15 @@ void GNUELFDumper::printRelRelaReloc(const
Relocation ,
template
static void printRelocHeaderFields(formatted_raw_ostream , unsigned SType,
- const typename ELFT::Ehdr ) {
+ const
@@ -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
@@ -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
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
@@ -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.
@@ -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
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
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
@@ -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:
> (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
@@ -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 ) {
jh7370 wrote:
Nit: `os` -> `OS`
@@ -259,7 +260,7 @@ class ELFObjectWriter : public MCObjectWriter {
void recordRelocation(MCAssembler , const MCAsmLayout ,
const MCFragment *Fragment, const MCFixup ,
MCValue Target, uint64_t ) override;
- bool usesRela(const
@@ -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
+
@@ -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
@@ -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.
@@ -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
@@ -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
@@ -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")?
@@ -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.
@@ -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 ) {
+ uint OffsetMask = 8, Offset = 0, Addend = 0;
+ uint32_t Symidx
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
@@ -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.
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
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
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
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
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
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
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
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
@@ -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
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://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
@@ -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
@@ -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
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
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
@@ -106,6 +126,210 @@ Error XCOFFDumper::dumpSections(ArrayRef Sections) {
return Error::success();
}
+Error XCOFFDumper::dumpFileAuxSym(XCOFFYAML::Symbol ,
+ const XCOFFSymbolRef ) {
+ for (uint8_t I = 1; I <= Sym.NumberOfAuxEntries; ++I)
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
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://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
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
@@ -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:
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 Type
@@ -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
@@ -2635,9 +2635,36 @@ template Error ELFWriter::finalize() {
}
Error BinaryWriter::write() {
- for (const SectionBase : Obj.allocSections())
+ SmallVector LoadableSections;
+ for (const SectionBase : Obj.allocSections()) {
+if (Sec.Type != SHT_NOBITS)
+
@@ -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
@@ -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
@@ -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
@@ -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
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
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.
@@ -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
@@ -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");
+
+
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
@@ -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");
+
+
@@ -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:
@@ -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
@@ -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
+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
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
Author: jhenderson
Date: Tue Sep 10 01:48:33 2019
New Revision: 371497
URL: http://llvm.org/viewvc/llvm-project?rev=371497=rev
Log:
Revert Remove REQUIRES:shell from tests that pass for me on Windows
This reverts r371478 (git commit a9980f60ce083fa6d5fd03c12c58ca0b293e3d60)
Modified:
77 matches
Mail list logo
