Page MenuHomePhabricator

grimar (George Rimar)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 21 2015, 12:36 AM (278 w, 4 d)

Recent Activity

Today

grimar added inline comments to D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
Fri, Jan 22, 5:30 AM · Restricted Project
grimar updated the diff for D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
  • Addressed review comments.
Fri, Jan 22, 5:30 AM · Restricted Project
grimar abandoned D93678: [yaml2obj] - Support selecting the location of the section header table..

D95140 was posted instead.

Fri, Jan 22, 4:53 AM · Restricted Project
grimar updated the diff for D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
  • Addressed review comments.
Fri, Jan 22, 4:50 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Fri, Jan 22, 3:57 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Fri, Jan 22, 3:56 AM · Restricted Project
grimar requested review of D95219: [libObject,llvm-readelf/obj] - Don't use @@ when printing versions of undefined symbols..
Fri, Jan 22, 3:28 AM · Restricted Project
grimar added a comment to D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..

It feels like there needs to be testing there for writing the section header table in the right place in the YAML (with Offset if needed), but I don't see that yet.

Fri, Jan 22, 1:46 AM · Restricted Project

Yesterday

grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Thu, Jan 21, 11:13 PM · Restricted Project
grimar requested review of D95140: [yaml2obj, obj2yaml] - Implement section header table as a special Chunk..
Thu, Jan 21, 7:45 AM · Restricted Project
grimar accepted D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.

LGTM with a nit, please wait for others.

Thu, Jan 21, 4:54 AM · Restricted Project
grimar added a comment to D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.

missing check-prefix. Not sure how I didn't spot the latter...

Thu, Jan 21, 4:42 AM · Restricted Project
grimar added inline comments to D95124: [lld][ELF][test] Add testing for IE/LD TLS weak undef references.
Thu, Jan 21, 4:16 AM · Restricted Project
grimar committed rGdd5c98280473: [llvm-nm][ELF] - Make -D display symbol versions. (authored by grimar).
[llvm-nm][ELF] - Make -D display symbol versions.
Thu, Jan 21, 12:25 AM
grimar closed D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Thu, Jan 21, 12:24 AM · Restricted Project

Wed, Jan 20

grimar committed rG51f4958057d6: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections. (authored by grimar).
[yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections.
Wed, Jan 20, 11:52 PM
grimar closed D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
Wed, Jan 20, 11:51 PM · Restricted Project
grimar accepted D94560: [ELF] report section sizes when output file too large.

This LGTM, but please wait for others.

Wed, Jan 20, 11:29 PM · Restricted Project
grimar added a comment to D94907: [llvm-nm][ELF] - Make -D display symbol versions..

@MaskRay, are you OK with it?

Wed, Jan 20, 12:32 AM · Restricted Project

Tue, Jan 19

grimar added a comment to D93761: [libObject/Decompressor] - Use `resize_for_overwrite` in Decompressor::resizeAndDecompress()..

Ping.

Tue, Jan 19, 7:05 AM · Restricted Project
grimar updated the diff for D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
  • Added test cases.
Tue, Jan 19, 7:05 AM · Restricted Project
grimar added inline comments to D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Tue, Jan 19, 6:11 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Addressed review comment.
Tue, Jan 19, 6:06 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Tue, Jan 19, 5:51 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Tue, Jan 19, 2:26 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Addressed review comments.
Tue, Jan 19, 2:26 AM · Restricted Project
grimar requested review of D94956: [yaml2obj/obj2yaml] - Improve dumping/creating of ELF versioning sections..
Tue, Jan 19, 2:05 AM · Restricted Project

Mon, Jan 18

grimar requested review of D94912: [llvm-nm][ELF] - Use @@ prefix when printing default versions..
Mon, Jan 18, 5:44 AM · Restricted Project
grimar updated the diff for D94907: [llvm-nm][ELF] - Make -D display symbol versions..
  • Format.
Mon, Jan 18, 4:33 AM · Restricted Project
grimar added inline comments to D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Mon, Jan 18, 4:31 AM · Restricted Project
grimar requested review of D94907: [llvm-nm][ELF] - Make -D display symbol versions..
Mon, Jan 18, 4:29 AM · Restricted Project
grimar committed rGb9ce772b8fb5: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h (authored by grimar).
[Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h
Mon, Jan 18, 1:51 AM
grimar closed D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Mon, Jan 18, 1:50 AM · Restricted Project
grimar updated the diff for D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
  • Addressed review comments.
Mon, Jan 18, 12:58 AM · Restricted Project
grimar updated the summary of D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Mon, Jan 18, 12:54 AM · Restricted Project
grimar added inline comments to D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Mon, Jan 18, 12:49 AM · Restricted Project
grimar added a comment to D94560: [ELF] report section sizes when output file too large.

Thanks again.

I chose DAG on purpose because I think it more accurately expresses what we're really asserting.

First, we since we removed the sorting, we're not guaranteeing that these will be in any particular order. Given that, I would rather not make the test depend on the order.

Mon, Jan 18, 12:22 AM · Restricted Project

Fri, Jan 15

grimar requested review of D94771: [Object, llvm-readelf] - Move the API for retrieving symbol versions to ELF.h.
Fri, Jan 15, 5:48 AM · Restricted Project
grimar committed rG45ef053bd709: [llvm-readobj][test] - Remove excessive YAML fields from tests. (authored by grimar).
[llvm-readobj][test] - Remove excessive YAML fields from tests.
Fri, Jan 15, 1:53 AM
grimar closed D94660: [llvm-readobj][test] - Remove excessive YAML fields from tests..
Fri, Jan 15, 1:53 AM · Restricted Project
grimar committed rGd9afe8588e49: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections. (authored by grimar).
[yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections.
Fri, Jan 15, 1:42 AM
grimar closed D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..
Fri, Jan 15, 1:42 AM · Restricted Project
grimar committed rG021ea78a97ed: [llvm-nm] - Simplify the code in dumpSymbolNamesFromObject. NFC. (authored by grimar).
[llvm-nm] - Simplify the code in dumpSymbolNamesFromObject. NFC.
Fri, Jan 15, 1:30 AM
grimar closed D94669: [llvm-nm] - Simplify the code in dumpSymbolNamesFromObject. NFC..
Fri, Jan 15, 1:30 AM · Restricted Project
grimar committed rGbfb8f45ef3f4: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). (authored by grimar).
[llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject().
Fri, Jan 15, 1:19 AM
grimar closed D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..
Fri, Jan 15, 1:19 AM · Restricted Project
grimar committed rG1185d3f43d21: [llvm-readobj] - Fix the compilation with GCC < 7.0. (authored by grimar).
[llvm-readobj] - Fix the compilation with GCC < 7.0.
Fri, Jan 15, 12:59 AM
grimar added a comment to D94560: [ELF] report section sizes when output file too large.

An alternative to this could probably be changing the Writer<ELFT>::assignFileOffsets()
It has the following loop:

Fri, Jan 15, 12:44 AM · Restricted Project
grimar added a comment to D94560: [ELF] report section sizes when output file too large.

Thanks for the comments!

E.g. I think it can be just a 2 lines loop which would iterate over all output sections and print their names ans sizes.
No need to sort them or to calculate fields width.

The reason I do the sorting is because there can potentially be very many sections. Imagine a large binary with -fdata-sections -ffunction-settings.

Fri, Jan 15, 12:29 AM · Restricted Project
grimar added inline comments to D93900: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..
Fri, Jan 15, 12:22 AM · Restricted Project
grimar updated the summary of D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..
Fri, Jan 15, 12:08 AM · Restricted Project
grimar added a comment to D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..

But I think using 0 in obj2yaml is fine.

Fri, Jan 15, 12:07 AM · Restricted Project

Thu, Jan 14

grimar added inline comments to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.
Thu, Jan 14, 11:51 PM · Restricted Project
grimar added a comment to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.

For the case to test error message when sh_size % sh_entsize is not 0, I didn't find any good examples that I can manipulate the sh_entsize through yaml2obj. Do you have good suggestions to write unit tests for these cases?

Thu, Jan 14, 11:46 PM · Restricted Project
grimar added a comment to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.

The reason is that .strtab and .shstrtab are placed at the end of elf file automatically by yaml2obj and there will almost always be cases to make (*It) & 1 != 0, preventing the code from reach the error message. I think the only way would be placing .gnu.hash at the end of the ELF, and remove the terminator, but that seems to be not possible with yaml2obj. For the case to test error message when sh_size % sh_entsize is not 0, I didn't find any good examples that I can manipulate the sh_entsize through yaml2obj. Do you have good suggestions to write unit tests for these cases?

Thu, Jan 14, 11:37 PM · Restricted Project
grimar added a comment to D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..

Does it have to stay in llvm-nm.cpp? A different file for MachO stuff?

Thu, Jan 14, 5:31 AM · Restricted Project
grimar added inline comments to D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..
Thu, Jan 14, 4:28 AM · Restricted Project
grimar added inline comments to D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..
Thu, Jan 14, 4:27 AM · Restricted Project
grimar added inline comments to D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..
Thu, Jan 14, 4:15 AM · Restricted Project
grimar updated the diff for D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..
  • Addressed review comments.
Thu, Jan 14, 4:15 AM · Restricted Project
grimar requested review of D94669: [llvm-nm] - Simplify the code in dumpSymbolNamesFromObject. NFC..
Thu, Jan 14, 3:52 AM · Restricted Project
grimar updated the summary of D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..
Thu, Jan 14, 3:36 AM · Restricted Project
grimar requested review of D94667: [llvm-nm] - Move MachO specific logic out from the dumpSymbolNamesFromObject(). NFC..
Thu, Jan 14, 3:35 AM · Restricted Project
grimar requested review of D94660: [llvm-readobj][test] - Remove excessive YAML fields from tests..
Thu, Jan 14, 1:36 AM · Restricted Project
grimar requested review of D94659: [yaml2obj/obj2yaml] - Refine handling of SHT_GNU_verdef sections..
Thu, Jan 14, 1:31 AM · Restricted Project
grimar added a comment to D94560: [ELF] report section sizes when output file too large.

FWIW, I think we could support this, but I'd implement it in a much more trivial way.
E.g. I think it can be just a 2 lines loop which would iterate over all output sections and print their names ans sizes.
No need to sort them or to calculate fields width.

Thu, Jan 14, 12:05 AM · Restricted Project

Wed, Jan 13

grimar added inline comments to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.
Wed, Jan 13, 11:57 PM · Restricted Project
grimar added a comment to D94620: [NFC] Disallow unused prefixes under MC/ARM.

I am not the right person to review this, I don't work on MC or MC/ARM. I think you should add people who mantains MC/ARM and/or may be authors of these tests.
The change looks good to me though.

Wed, Jan 13, 11:41 PM · Restricted Project
grimar added a comment to D93761: [libObject/Decompressor] - Use `resize_for_overwrite` in Decompressor::resizeAndDecompress()..

Ok. I've used the following piece of code to construct a 1Gb of random compressed data and estimated
the difference between the old and the new code in Release.

Wed, Jan 13, 4:11 AM · Restricted Project
grimar added a comment to D93678: [yaml2obj] - Support selecting the location of the section header table..

An alternative would be to move the Offset aspect of the sections into the Layout table only, and possibly moving Fills into that Layout block too out of the Sections list. This has the advantage that layout information is generally kept in the Layout table, whilst the details of what a thing actually is (e.g. section type, size, address etc) are kept in appropriate parts of the document. This is closer to my original option 3:

FileHeader:
  ...
Sections:
  - Name: .foo
    Type: SHT_PROGBITS
  - Name: .bar
    Type: SHT_PROGBITS
SectionHeaderTable:
  ...
ProgramHeaders:
  ...
Layout:
   - Type: Section
    Name: .foo
    Offset: 0x1000
  - Type: Fill
    # Offset could be implicitly calculated here; fills move out of Sections to Layout.
    Size: 0x100
  - Type: SectionHeaderTable
    Offset: 0x2000
  - Type: ProgramHeaderTable
    Offset: 0x3000
  - Type: Section
    Name: .bar
    Offset: 0x3500

I think I mildly prefer the second one, as it'll be easier to retrofit and feels a little cleaner. We'd need some implicit rules that the program header table appears at the front and the section header table at the back if they aren't specifically mentioned in either case.

Wed, Jan 13, 1:47 AM · Restricted Project
grimar committed rG6d3098e7ff96: [obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field. (authored by grimar).
[obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field.
Wed, Jan 13, 12:53 AM
grimar closed D93858: [obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field..
Wed, Jan 13, 12:53 AM · Restricted Project
grimar committed rG141906fa149f: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections. (authored by grimar).
[llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections.
Wed, Jan 13, 12:37 AM
grimar closed D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
Wed, Jan 13, 12:37 AM · Restricted Project

Tue, Jan 12

grimar added a comment to D93678: [yaml2obj] - Support selecting the location of the section header table..

Option 3: Have a separate "Layout" block within the YAML which would allow you to define the layout. It might look a bit like this:

Layout:
  - FileHeader
  - Section1
  - ProgramHeaders
  - Section2
  - SectionHeaders
  - Section3

I'm not sure how this option would work alongside Offset values for sections, or what should happen if e.g. the file header entry were to be omitted.

Tue, Jan 12, 6:46 AM · Restricted Project
grimar added inline comments to D93858: [obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field..
Tue, Jan 12, 6:16 AM · Restricted Project
grimar updated the diff for D93858: [obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field..
  • Addressed review comments.
Tue, Jan 12, 6:16 AM · Restricted Project
grimar added inline comments to D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
Tue, Jan 12, 6:09 AM · Restricted Project
grimar added a comment to D93858: [obj2yaml,yaml2obj] - Refine how we set/dump the sh_entsize field..

Could you explain why this patch fixes the ARM_EXIDX sh_entsize specifically? I don't see anything immediately that would only fix that section (and not other sections).

Tue, Jan 12, 5:14 AM · Restricted Project
grimar added inline comments to D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
Tue, Jan 12, 5:03 AM · Restricted Project
grimar updated the diff for D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
  • Rebased after landing the "[llvm-readef/obj] - Change the design structure of ELF dumper. NFCI." (D93900).
  • Addressed review comments.
Tue, Jan 12, 5:03 AM · Restricted Project
grimar committed rGc15a57cc1a86: [obj2yaml] - Don't crash when an object has an empty symbol table. (authored by grimar).
[obj2yaml] - Don't crash when an object has an empty symbol table.
Tue, Jan 12, 3:17 AM
grimar closed D93697: [obj2yaml] - Don't crash when an object has an empty symbol table..
Tue, Jan 12, 3:16 AM · Restricted Project
grimar committed rG60df7c08b1f4: [obj2yaml,yaml2obj] - Fix issues with creating/dumping group sections. (authored by grimar).
[obj2yaml,yaml2obj] - Fix issues with creating/dumping group sections.
Tue, Jan 12, 3:08 AM
grimar closed D93854: [obj2yaml,yaml2obj] - Fix issues with creating/dumping group sections..
Tue, Jan 12, 3:08 AM · Restricted Project
grimar added inline comments to rG9ec72cfc61ad: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..
Tue, Jan 12, 2:25 AM
grimar committed rG891b4873c129: [llvm-readobj] - One more attempt to fix BB. (authored by grimar).
[llvm-readobj] - One more attempt to fix BB.
Tue, Jan 12, 2:18 AM
grimar added inline comments to rG9ec72cfc61ad: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..
Tue, Jan 12, 2:16 AM
grimar committed rGcc91efdabee0: [llvm-readobj] - An attempt to fix BB. (authored by grimar).
[llvm-readobj] - An attempt to fix BB.
Tue, Jan 12, 2:10 AM
grimar committed rG1e11402aa8e2: [llvm-readobj] - Add 'override' to fix build bots. (authored by grimar).
[llvm-readobj] - Add 'override' to fix build bots.
Tue, Jan 12, 2:02 AM
grimar committed rG9ec72cfc61ad: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI. (authored by grimar).
[llvm-readef/obj] - Change the design structure of ELF dumper. NFCI.
Tue, Jan 12, 1:52 AM
grimar closed D93900: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..
Tue, Jan 12, 1:52 AM · Restricted Project
grimar added inline comments to D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
Tue, Jan 12, 1:33 AM · Restricted Project
grimar updated the diff for D92923: [llvm-readelf/obj] - Add support of multiple SHT_SYMTAB_SHNDX sections..
  • Addressed review comments.
  • Added the unit test.
Tue, Jan 12, 1:33 AM · Restricted Project
grimar added inline comments to D93362: [llvm-elfabi] Support ELF file that lacks .gnu.hash section.
Tue, Jan 12, 12:52 AM · Restricted Project
grimar added a comment to D94354: [ELF] Drop .rel[a].debug_gnu_pub{names,types} for --gdb-index --emit-relocs.

Still LG.

Tue, Jan 12, 12:06 AM · Restricted Project

Mon, Jan 11

grimar accepted D94354: [ELF] Drop .rel[a].debug_gnu_pub{names,types} for --gdb-index --emit-relocs.

LGTM

Mon, Jan 11, 11:49 PM · Restricted Project
grimar added a comment to D93900: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..

Could you highlight the major changes you've had to make to achieve the refactor? This patch is quite large and tricky for me to review in depth.

Mon, Jan 11, 6:09 AM · Restricted Project
grimar updated the diff for D93900: [llvm-readef/obj] - Change the design structure of ELF dumper. NFCI..
  • Addressed review comments.
Mon, Jan 11, 6:09 AM · Restricted Project
grimar updated the diff for D93697: [obj2yaml] - Don't crash when an object has an empty symbol table..
  • Updated code and test case comments to address suggestions from reviewers.
Mon, Jan 11, 5:31 AM · Restricted Project