zturner (Zachary Turner)
User

Projects

User does not belong to any projects.

User Details

User Since
May 26 2014, 12:49 PM (165 w, 2 h)

Recent Activity

Fri, Jul 21

zturner added a comment to D35738: Enable llvm-pdbutil to list enumerations using native PDB reader.

Is it that much extra work to implement children? It would be nice if this could just be feature complete. It seems like all you have to do is read the FieldList member of the CodeView record, load that item, and then create an enumerator that iterates each item and returns a NativeConstantSymbol or whatever we're calling it

Fri, Jul 21, 7:11 PM

Tue, Jul 11

zturner added a comment to D35290: [pdb/lld] Add an empty globals stream.

Note that this still doesn't get our PDBs working with MSVC. next steps:

Tue, Jul 11, 7:56 PM
zturner created D35290: [pdb/lld] Add an empty globals stream.
Tue, Jul 11, 7:50 PM
zturner added inline comments to D35163: [PDB] Enable NativeSession to create symbols for built-in types on demand.
Tue, Jul 11, 10:28 AM
zturner added a comment to D35163: [PDB] Enable NativeSession to create symbols for built-in types on demand.

Ok I think I get it. There's a distinction between "sym index id" (DIA terminology) and "type index" (codeview terminology) that I think is getting lost here. Can we make a typedef for SymIndexId (call it something else if you need to, since that would conflict with DIA typedefs. Like PDB_SymId. Then have this function return a PDB_SymId and accept a codeview::TypeIndex?

Tue, Jul 11, 10:27 AM
zturner added inline comments to D35163: [PDB] Enable NativeSession to create symbols for built-in types on demand.
Tue, Jul 11, 10:07 AM
zturner added inline comments to D35163: [PDB] Enable NativeSession to create symbols for built-in types on demand.
Tue, Jul 11, 10:05 AM

Mon, Jul 10

zturner committed rL307598: [lld/pdb] Create an empty public symbol record stream..
[lld/pdb] Create an empty public symbol record stream.
Mon, Jul 10, 3:40 PM
zturner closed D35224: [lld/pdb] Create an empty public symbol records stream by committing rL307598: [lld/pdb] Create an empty public symbol record stream..
Mon, Jul 10, 3:40 PM
zturner created D35224: [lld/pdb] Create an empty public symbol records stream.
Mon, Jul 10, 3:03 PM
zturner committed rL307592: Fix pdb-linker-module test on linux..
Fix pdb-linker-module test on linux.
Mon, Jul 10, 2:09 PM
zturner committed rL307590: [lld/pdb] Add some basic linker module symbols..
[lld/pdb] Add some basic linker module symbols.
Mon, Jul 10, 2:02 PM
zturner closed D35152: Add some basic linker module symbols by committing rL307590: [lld/pdb] Add some basic linker module symbols..
Mon, Jul 10, 2:02 PM
zturner added inline comments to D35152: Add some basic linker module symbols.
Mon, Jul 10, 2:01 PM
zturner updated the diff for D35152: Add some basic linker module symbols.

Updated the ELF side.

Mon, Jul 10, 1:38 PM
zturner added a comment to D35152: Add some basic linker module symbols.

Should I still leave the variable in Error.h? It seems a little illogical to have the variable in Error.h given that it's now being used outside of Error related code. Config.h seems like the logical place for it. Maybe the best thing to do is add the vector in Configuration of ELF linker too?

Mon, Jul 10, 1:27 PM
zturner added inline comments to D35152: Add some basic linker module symbols.
Mon, Jul 10, 1:22 PM
zturner added inline comments to D35152: Add some basic linker module symbols.
Mon, Jul 10, 1:16 PM
zturner added inline comments to D35152: Add some basic linker module symbols.
Mon, Jul 10, 1:14 PM
zturner updated the diff for D35152: Add some basic linker module symbols.

Update the test to properly check for x64, and finish up the remaining missing pieces. In particular, we need the value of argv[0] in order to portably get the executable name, and we need the values of argv[1] - argv[argc-1] in order to write them to the S_ENVBLOCK symbol. To address this, I deleted the global Argv0 variable that LLD currently used, and instead stored the entire argv array in the global configuration object.

Mon, Jul 10, 12:50 PM
zturner committed rL307571: Resubmit "Add pdb-diff test.".
Resubmit "Add pdb-diff test."
Mon, Jul 10, 12:17 PM
zturner added inline comments to D35152: Add some basic linker module symbols.
Mon, Jul 10, 10:59 AM
zturner committed rL307559: Revert "Build fixes for pdb-diff test.".
Revert "Build fixes for pdb-diff test."
Mon, Jul 10, 10:33 AM
zturner committed rL307556: Build fixes for pdb-diff test..
Build fixes for pdb-diff test.
Mon, Jul 10, 10:01 AM
zturner committed rL307555: Fix pdb-diff test..
Fix pdb-diff test.
Mon, Jul 10, 9:52 AM
zturner committed rL306833: Revert "[lit] Clean output directories before running tests.".
Revert "[lit] Clean output directories before running tests."
Mon, Jul 10, 9:42 AM
zturner committed rL306832: [lit] Clean output directories before running tests..
[lit] Clean output directories before running tests.
Mon, Jul 10, 9:42 AM

Sun, Jul 9

zturner committed rL307512: Don't access Python objects while not holding the GIL..
Don't access Python objects while not holding the GIL.
Sun, Jul 9, 4:32 PM
zturner closed D34942: [PYTHON] Fix crash with "Fatal Python error: Python memory allocator called without holding the GIL" by committing rL307512: Don't access Python objects while not holding the GIL..
Sun, Jul 9, 4:32 PM
zturner committed rL307459: Delete the pdb diff test..
Delete the pdb diff test.
Sun, Jul 9, 6:19 AM
zturner committed rL307436: [PDB] More changes to bring lld PDBs to parity with MSVC..
[PDB] More changes to bring lld PDBs to parity with MSVC.
Sun, Jul 9, 6:17 AM
zturner committed rL307426: [llvm-pdbutil] Fix build..
[llvm-pdbutil] Fix build.
Sun, Jul 9, 6:16 AM
zturner committed rL307422: Fix some differences between lld and MSVC generated PDBs..
Fix some differences between lld and MSVC generated PDBs.
Sun, Jul 9, 6:16 AM
zturner committed rL307423: Use windows path syntax when writing PDB module name..
Use windows path syntax when writing PDB module name.
Sun, Jul 9, 6:16 AM
zturner committed rL307421: [llvm-pdbutil] Improve diff mode..
[llvm-pdbutil] Improve diff mode.
Sun, Jul 9, 6:16 AM
zturner closed D35086: Fix some differences between LLD and MSVC generated PDBs by committing rL307422: Fix some differences between lld and MSVC generated PDBs..
Sun, Jul 9, 6:16 AM
zturner closed D35092: Use native path syntax when writing PDB module name. by committing rL307423: Use windows path syntax when writing PDB module name..
Sun, Jul 9, 6:16 AM
zturner closed D35039: [llvm-pdbutil] Improve output of llvm-pdbutil diff mode by committing rL307421: [llvm-pdbutil] Improve diff mode..
Sun, Jul 9, 6:16 AM
zturner created D35152: Add some basic linker module symbols.
Sun, Jul 9, 6:13 AM
zturner accepted D35099: Add name offset flags, for parity with cvtres.exe..
Sun, Jul 9, 6:13 AM

Fri, Jul 7

zturner added inline comments to D35039: [llvm-pdbutil] Improve output of llvm-pdbutil diff mode.
Fri, Jul 7, 10:15 AM
zturner added inline comments to D35092: Use native path syntax when writing PDB module name..
Fri, Jul 7, 9:58 AM
zturner added inline comments to D35086: Fix some differences between LLD and MSVC generated PDBs.
Fri, Jul 7, 9:54 AM
zturner added a comment to D35086: Fix some differences between LLD and MSVC generated PDBs.

(BTW, ignore in case you already noticed them, but I also have D35092 and D35039 if you don't mind taking a look at those as well)

Fri, Jul 7, 9:53 AM

Thu, Jul 6

zturner committed rL307360: Fix lld tests after r307356..
Fix lld tests after r307356.
Thu, Jul 6, 10:42 PM
zturner committed rL307356: [PDB] Teach libpdb to write DBI Stream ECNames..
[PDB] Teach libpdb to write DBI Stream ECNames.
Thu, Jul 6, 10:05 PM
zturner created D35092: Use native path syntax when writing PDB module name..
Thu, Jul 6, 2:54 PM
zturner created D35086: Fix some differences between LLD and MSVC generated PDBs.
Thu, Jul 6, 2:16 PM
zturner added a comment to D35072: Stop defining `PAGE_SIZE`, and use `getpagesize()` on Unix.

You'll probably need to #include <windows.h>. As I'm not familiar with this code, there may be issues with including windows.h from a header which I'm not aware of (it doesn't appear to be included in this header file). (also, it's SYSTEM_INFO, not SYSTEMINFO)

Thu, Jul 6, 11:01 AM
zturner added a comment to D35072: Stop defining `PAGE_SIZE`, and use `getpagesize()` on Unix.

There's a comment indicating that we should find the correct function on Windows and use it, but in the description of this review you mention that you found it in the SYSTEM_INFO structure. So why not use it?

Thu, Jul 6, 10:36 AM
zturner accepted D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records.
Thu, Jul 6, 9:01 AM

Wed, Jul 5

zturner added inline comments to D34915: [pdb] Add a test for every known CodeView type.
Wed, Jul 5, 5:02 PM
zturner created D35039: [llvm-pdbutil] Improve output of llvm-pdbutil diff mode.
Wed, Jul 5, 4:53 PM
zturner committed rL307205: Fix std::min ambiguity between uint32 and size_t..
Fix std::min ambiguity between uint32 and size_t.
Wed, Jul 5, 2:59 PM
zturner committed rL307204: [llvm-pdbutil] Add the ability to truncate stream purpose names..
[llvm-pdbutil] Add the ability to truncate stream purpose names.
Wed, Jul 5, 2:55 PM
zturner committed rL307187: [PDB] Add a test that verifies every known type record..
[PDB] Add a test that verifies every known type record.
Wed, Jul 5, 11:44 AM
zturner closed D34915: [pdb] Add a test for every known CodeView type by committing rL307187: [PDB] Add a test that verifies every known type record..
Wed, Jul 5, 11:44 AM
zturner accepted D34976: [lit] Fix unit test discovery for Visual Studio builds..
Wed, Jul 5, 10:48 AM

Fri, Jun 30

zturner added a comment to D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records.

Did the new patch not get uploaded? I'm still seeing the original patch.

Fri, Jun 30, 5:02 PM
zturner added inline comments to D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler..
Fri, Jun 30, 4:56 PM
zturner added inline comments to D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler..
Fri, Jun 30, 4:45 PM
zturner added a comment to D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler..

Also, I think most (all?) of the original comments I had still apply.

Fri, Jun 30, 4:31 PM
zturner added a comment to D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler..

Will add more comments later.

Fri, Jun 30, 4:31 PM
zturner created D34915: [pdb] Add a test for every known CodeView type.
Fri, Jun 30, 3:40 PM
zturner committed rL306891: Remove spurious semicolons..
Remove spurious semicolons.
Fri, Jun 30, 2:49 PM
zturner committed rL306890: [llvm-pdbutil] Output the symbol offset when dumping..
[llvm-pdbutil] Output the symbol offset when dumping.
Fri, Jun 30, 2:35 PM
zturner closed D34906: [llvm-pdbutil] Output the symbol offset when dumping symbols by committing rL306890: [llvm-pdbutil] Output the symbol offset when dumping..
Fri, Jun 30, 2:35 PM
zturner added inline comments to D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records.
Fri, Jun 30, 2:27 PM
zturner added inline comments to D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records.
Fri, Jun 30, 2:26 PM
zturner added inline comments to D34906: [llvm-pdbutil] Output the symbol offset when dumping symbols.
Fri, Jun 30, 2:13 PM
zturner accepted D34907: fix ODR violations due to abuse of LLVM_YAML_IS_(FLOW_)?SEQUENCE_VECTOR.

I don't have any comments, this seems fine to me. Long term I think the best solution is to get rid of these global specializations entirely, and instead provide adapters called flow and block, and instead of just saying IO.mapOptional("Foo", Sequence); and having it be picked up by a specialization, you would say IO.mapOptional("Foo", flow(Sequence));

Fri, Jun 30, 1:51 PM
zturner created D34906: [llvm-pdbutil] Output the symbol offset when dumping symbols.
Fri, Jun 30, 1:35 PM
zturner added inline comments to D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records.
Fri, Jun 30, 11:42 AM
zturner committed rL306856: Fix test broken by parameter mixup..
Fix test broken by parameter mixup.
Fri, Jun 30, 11:25 AM
zturner committed rL306852: [llvm-pdbutil] Add the ability to dump the dependency tree for a type.
[llvm-pdbutil] Add the ability to dump the dependency tree for a type
Fri, Jun 30, 11:16 AM
zturner closed D34899: [llvm-pdbutil] Add the ability to dump a single type's entire dependency chain by committing rL306852: [llvm-pdbutil] Add the ability to dump the dependency tree for a type.
Fri, Jun 30, 11:16 AM
zturner created D34899: [llvm-pdbutil] Add the ability to dump a single type's entire dependency chain.
Fri, Jun 30, 10:34 AM
zturner closed D34732: Clean temp directories before running lit by committing rL306832: [lit] Clean output directories before running tests..
Fri, Jun 30, 9:01 AM

Thu, Jun 29

zturner added a comment to D34600: [Error] add expectSuccess().

I think we should minimize use of report_fatal_error in library builds. This seems like a textbook assert use case to me. No different than assert(foo != nullptr); The only objection I have to having both a check and an assert method (and it is a minor object, mind you) is YAGNI.

Thu, Jun 29, 10:22 AM
zturner accepted D34744: [DWARF] - Simplify HandleExpectedError implementation in DWARFDebugInfoTest.

lgtm. I have some other ideas for how to improve this test in the future, but mostly orthogonal to this effort (for example, why can't we just enumerate the cases that we expect to work based on the TargetRegistry, and then only run those tests?)

Thu, Jun 29, 10:16 AM
zturner updated the diff for D34732: Clean temp directories before running lit.

Update with suggestions from rnk. With this patch all tests pass.

Thu, Jun 29, 10:08 AM

Wed, Jun 28

zturner updated the diff for D34732: Clean temp directories before running lit.

Updated with fixes from dblaikie. All tests pass now.

Wed, Jun 28, 3:30 PM
zturner added a comment to D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler..

More comments later when I have some time to look in more detail.

Wed, Jun 28, 1:21 PM
zturner added a comment to D34764: Introduce FileEdit utility.
In D34764#794086, @rnk wrote:

@chandlerc I think we have use cases for file splitting in the linker. Many interesting test cases require two object files, and it's annoying to have to make a separate file in %S/Inputs just for this. Consider lld/test/COFF/reloc-discarded.s. I recently added this and I need to make a COFF object that defines a global in a comdat. This is the RUN: line I used to do that:

# RUN: echo -e '.section .bss,"bw",discard,main_global\n.global main_global\n main_global:\n .long 0' | \
# RUN:     llvm-mc - -filetype=obj -o %t1.obj -triple x86_64-windows-msvc

That's pretty unreadable.

FWIW, I agree this is awful.

I'm not as fussed as you seem to be about having separate files... but OK.

I would much rather write:

# RUN: FileEdit %s %t
# RUN: llvm-mc %t/a.s -o %t/a.obj -filetype=obj -triple x86_64-windows-msvc
# RUN: llvm-mc %t/b.s -o %t/b.obj -filetype=obj -triple x86_64-windows-msvc
# RUN: lld-link %t/a.obj %t/b.obj -entry ...

# SPLIT: a.s
.section .bss,"bw",discard,main_global
.globl main_global
main_global:
    .long 0

# SPLIT: b.s
.section .bss,"bw",discard,main_global
.globl main_global
main_global:
    .long 0
...

Editors would enable assembly syntax highlighting, even. You can apply the same technique to Clang tests that need headers.

I just don't think we need a new syntax or tool here.

We can run the preprocessor to create a header file, and then include it. We can use the preprocessor to have a single source file and compile it in N different ways.

We could even use preprocessed assembly to solve the issue you cite in the lld test suite. I'd honestly rather continue using the *existing* file manipulation syntaxes rather than adding yet another magic comment that actually does semantic splitting.

Wed, Jun 28, 11:36 AM
zturner added a comment to D34764: Introduce FileEdit utility.
In D34764#794038, @rnk wrote:

Why {!--? It seems inconsistent with the usual LLVM thing of an all caps directive like CHECK:, RUN:, or a Unixy-EOF. Unbalanced braces will make tools like clang-format misbehave.

Wed, Jun 28, 10:21 AM
zturner created D34764: Introduce FileEdit utility.
Wed, Jun 28, 10:12 AM
zturner added inline comments to D34744: [DWARF] - Simplify HandleExpectedError implementation in DWARFDebugInfoTest.
Wed, Jun 28, 8:10 AM
zturner added a comment to D34746: Move Timer and TraceOptions from Core to Utility.

Can you do a diff before and after of the analyze-deps script output? I don't imagine it will make a difference, but I've been surprised by it before. In general you should do it before and after every patch of this nature.

Wed, Jun 28, 8:04 AM

Tue, Jun 27

zturner added a comment to D34732: Clean temp directories before running lit.

More sinister is that leaked files from previous runs can lead to false positives in tests.

I've also seen this problem; thanks for working on a fix.

Does this always remove the entire Output directory if you're only running a subset of the tests?

Tue, Jun 27, 6:50 PM
zturner updated the diff for D34732: Clean temp directories before running lit.

Two files from an unrelated CL snuck in. Removed those.

Tue, Jun 27, 6:13 PM
zturner created D34732: Clean temp directories before running lit.
Tue, Jun 27, 6:11 PM
zturner accepted D34542: Introduce symbol cache to PDB NativeSession.
Tue, Jun 27, 3:03 PM
zturner added inline comments to D34683: [unittests] Add a helper function for getting an input file.
Tue, Jun 27, 2:40 PM
zturner added inline comments to D34542: Introduce symbol cache to PDB NativeSession.
Tue, Jun 27, 1:29 PM

Mon, Jun 26

zturner committed rL306309: [llvm-pdbutil] Add a mode to `bytes` for dumping split debug chunks..
[llvm-pdbutil] Add a mode to `bytes` for dumping split debug chunks.
Mon, Jun 26, 10:23 AM
zturner accepted D34400: Move Connection from Core to Host.

Nice, yea this will be a big help I think.

Mon, Jun 26, 10:09 AM
zturner accepted D34625: Move StructuredData from Core to Utility.
Mon, Jun 26, 10:06 AM
zturner committed rL306233: [pdb] Fix reading of llvm-generated PDBs by cvdump..
[pdb] Fix reading of llvm-generated PDBs by cvdump.
Mon, Jun 26, 2:47 AM
zturner committed rL306230: [Support] Don't use std::iterator, it's deprecated in C++17..
[Support] Don't use std::iterator, it's deprecated in C++17.
Mon, Jun 26, 2:47 AM
zturner closed D34491: [PDB] Fix reading of clang-generated PDBs by CVDump. by committing rL306233: [pdb] Fix reading of llvm-generated PDBs by cvdump..
Mon, Jun 26, 2:47 AM