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
LO
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:
cf
@@ -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.NumberOfA
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
addition
@@ -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 ab
@@ -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 o
@@ -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:
P
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
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 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://l
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
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
@@ -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 usi
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
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
@@ -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.OutputForma
@@ -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
--check-pr
@@ -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-pr
@@ -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)
+
@@ -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.OutputForma
@@ -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
@@ -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-pr
@@ -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 (Config.OutputForma
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:
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
@@ -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
out
@@ -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
out
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 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 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 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-debu
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.llv
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.ll
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:
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 lis
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
y
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 thi
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 man
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/lis
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
@@ -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. Ind
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
@@ -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 Symi
@@ -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.
@@ -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")?
@@ -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 Hd
@@ -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 Hd
@@ -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
jh7370 wrote:
Endian/class ma
@@ -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
+
--
@@ -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) overr
@@ -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://githu
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
_
@@ -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
+
--
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
@@ -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 =
@@ -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.
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
@@ -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 h
@@ -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
@@ -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) {
+
@@ -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 wro
@@ -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 u
@@ -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
+
--
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
___
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,
@@ -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-
@@ -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 s
@@ -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:
Unnece
@@ -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");
+
+
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,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");
+
+
@@ -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
+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
77 matches
Mail list logo