zturner (Zachary Turner)
User

Projects

User does not belong to any projects.

User Details

User Since
May 26 2014, 12:49 PM (229 w, 1 d)

Recent Activity

Mon, Oct 15

zturner added a comment to D52461: [PDB] Introduce `PDBNameParser`.

Hello!

I just have tried to patch CPlusPlusNameParser in the way to support MSVC demangled names, but there is a problem. CPlusPlusNameParser splits an incoming name in tokens with clang::Lexer. I've lexed the next name:

`anonymous namespace'::foo

The lexer treats the first character (a grave accent) as an unknown token, and it's ok for our purposes. Then it sees an identifier (anonymous), a keyword (namespace), and it's ok too. But the problem is with the last part of the string. The lexer sees an apostrophe and supposes that it's a character constant, it looks for a closing apostrophe, don't find it and treats all the line ending ('::foo) as a single unknown token.

It is possible to somehow make clang::Lexer lex MSVC demangled names correctly, but I'm not sure if it is the right place to do it. And it may have then some side effects during lexing a real code.

Another option is to somehow preprocess the name before lexing and replace all paired apostrophes with grave accents, and after lexing replace with apostrophes back, and make CPlusPlusNameParser understand unknown grave accent tokens. But it's a bit tricky, may be you can suggest some better solution?

Mon, Oct 15, 10:53 AM · Restricted Project

Fri, Oct 12

zturner committed rLLDB344431: Add REQUIRES: lld to SymbolFileNativePDB tests..
Add REQUIRES: lld to SymbolFileNativePDB tests.
Fri, Oct 12, 4:09 PM
zturner committed rL344431: Add REQUIRES: lld to SymbolFileNativePDB tests..
Add REQUIRES: lld to SymbolFileNativePDB tests.
Fri, Oct 12, 4:09 PM
zturner committed rLLDB344429: Try to fix some failures on MacOSX with the NativePDB patch..
Try to fix some failures on MacOSX with the NativePDB patch.
Fri, Oct 12, 3:59 PM
zturner committed rL344429: Try to fix some failures on MacOSX with the NativePDB patch..
Try to fix some failures on MacOSX with the NativePDB patch.
Fri, Oct 12, 3:59 PM
zturner committed rLLDB344409: Resubmit "Add SymbolFileNativePDB plugin.".
Resubmit "Add SymbolFileNativePDB plugin."
Fri, Oct 12, 12:49 PM
zturner committed rL344409: Resubmit "Add SymbolFileNativePDB plugin.".
Resubmit "Add SymbolFileNativePDB plugin."
Fri, Oct 12, 12:49 PM
zturner accepted D53166: [lldbsuite] Fix the filecheck functionality to work with Python 3.
Fri, Oct 12, 10:51 AM
zturner committed rL344378: Fix another error related to YAML quoting..
Fix another error related to YAML quoting.
Fri, Oct 12, 10:31 AM
zturner committed rLLD344377: Better support for POSIX paths in PDBs..
Better support for POSIX paths in PDBs.
Fri, Oct 12, 10:28 AM
zturner committed rL344377: Better support for POSIX paths in PDBs..
Better support for POSIX paths in PDBs.
Fri, Oct 12, 10:28 AM
zturner committed rCTE344362: Fix one additional test broken by the YAML quoting change..
Fix one additional test broken by the YAML quoting change.
Fri, Oct 12, 9:43 AM
zturner committed rL344362: Fix one additional test broken by the YAML quoting change..
Fix one additional test broken by the YAML quoting change.
Fri, Oct 12, 9:43 AM
zturner committed rL344359: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:33 AM
zturner committed rC344359: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:33 AM
zturner committed rLLD344359: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:33 AM
zturner committed rL344358: Revert "Make YAML quote forward slashes.".
Revert "Make YAML quote forward slashes."
Fri, Oct 12, 9:33 AM
zturner closed D53169: Make YAML serializer quote forward slashes.
Fri, Oct 12, 9:33 AM
zturner committed rC344358: Revert "Make YAML quote forward slashes.".
Revert "Make YAML quote forward slashes."
Fri, Oct 12, 9:33 AM
zturner committed rLLD344358: Revert "Make YAML quote forward slashes.".
Revert "Make YAML quote forward slashes."
Fri, Oct 12, 9:33 AM
zturner committed rC344357: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:26 AM
zturner committed rLLD344357: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:26 AM
zturner committed rL344357: Make YAML quote forward slashes..
Make YAML quote forward slashes.
Fri, Oct 12, 9:26 AM

Thu, Oct 11

zturner added a comment to D53086: [PDB] Fix flaky `variables-locations.test` after PR38857.

Thanks a lot for so detailed answer, it helps!

So we need to parse a FPO program and to convert it in a DWARF expression too. The problem here (in the DIA case) is that I don't know how to retrieve the required FPO range (we have a symbol context when creating a variable, but it seems that it doesn't contain enough information).

I think the SymbolContext should have enough information. As long as you can find the PDBSymbolFunction for the current frame, that gives you the range of the function, and the IDiaSymbol for the variable should give you the important info like register, offset, etc.

Thu, Oct 11, 8:28 PM · Restricted Project
zturner added a comment to D53086: [PDB] Fix flaky `variables-locations.test` after PR38857.

Thanks a lot for so detailed answer, it helps!

So we need to parse a FPO program and to convert it in a DWARF expression too. The problem here (in the DIA case) is that I don't know how to retrieve the required FPO range (we have a symbol context when creating a variable, but it seems that it doesn't contain enough information).

I think the SymbolContext should have enough information. As long as you can find the PDBSymbolFunction for the current frame, that gives you the range of the function, and the IDiaSymbol for the variable should give you the important info like register, offset, etc.

Thu, Oct 11, 8:26 PM · Restricted Project
zturner accepted D53179: [codeview] Emit S_BUILDINFO and LF_BUILDINFO with cwd and source file.
Thu, Oct 11, 5:49 PM
zturner updated subscribers of D53175: [dotest] Make a missing FileCheck binary a warning, not an error.

Why is FileCheck missing in the first place?

Thu, Oct 11, 3:23 PM
zturner updated subscribers of D50478: Add support for artificial tail call frames.

Is the thread I'm referring to.

Thu, Oct 11, 3:01 PM
zturner updated subscribers of D50478: Add support for artificial tail call frames.

See the other email thread. The bots have been broken since September, all
of them complain about missing FileCheck executable

Thu, Oct 11, 2:59 PM
zturner created D53169: Make YAML serializer quote forward slashes.
Thu, Oct 11, 2:00 PM
zturner added inline comments to D53166: [lldbsuite] Fix the filecheck functionality to work with Python 3.
Thu, Oct 11, 1:51 PM
zturner committed rLLD344279: Revert SymbolFileNativePDB plugin..
Revert SymbolFileNativePDB plugin.
Thu, Oct 11, 11:48 AM
zturner committed rL344279: Revert SymbolFileNativePDB plugin..
Revert SymbolFileNativePDB plugin.
Thu, Oct 11, 11:48 AM
zturner committed rLLDB344279: Revert SymbolFileNativePDB plugin..
Revert SymbolFileNativePDB plugin.
Thu, Oct 11, 11:47 AM
zturner committed rLLDB344269: Better support for POSIX paths in PDBs..
Better support for POSIX paths in PDBs.
Thu, Oct 11, 11:05 AM
zturner committed rL344269: Better support for POSIX paths in PDBs..
Better support for POSIX paths in PDBs.
Thu, Oct 11, 11:05 AM
zturner committed rLLD344269: Better support for POSIX paths in PDBs..
Better support for POSIX paths in PDBs.
Thu, Oct 11, 11:04 AM
zturner closed D53149: [PDB] Better support for posix style paths in PDBs..
Thu, Oct 11, 11:04 AM
zturner created D53149: [PDB] Better support for posix style paths in PDBs..
Thu, Oct 11, 10:38 AM

Wed, Oct 10

zturner committed rL344216: Use fully qualified namespace name..
Use fully qualified namespace name.
Wed, Oct 10, 8:44 PM
zturner committed rLLDB344173: [SymbolFileNativePDB] Fix compilation errors with gcc..
[SymbolFileNativePDB] Fix compilation errors with gcc.
Wed, Oct 10, 11:56 AM
zturner committed rL344173: [SymbolFileNativePDB] Fix compilation errors with gcc..
[SymbolFileNativePDB] Fix compilation errors with gcc.
Wed, Oct 10, 11:55 AM
zturner added a comment to D52193: RFC: [clang] Multithreaded compilation support.

I can try to get some timings from my machine. How do we handle crash
recovery in the case where we don't spawn a child process? I thought the
whole reason for spawning the cc1 driver as a separate process was so that
we could collect and report crash information in a nice way. Not having
that seems like a heavy price to pay.

Wed, Oct 10, 11:55 AM
zturner added a comment to D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules().

If you plan to invest in more substantial changes in ObjectFilePECOFF, it might worth considering a complete re-write in terms of llvm::object::coff. It has pretty comprehensive support for the PE/COFF spec, so it's just a matter of implementing ObjectFilePECOFF on top of it.

Wed, Oct 10, 11:27 AM
zturner accepted D53090: [Windows] Fix a bug that causes lldb to freeze.

Nice find, thanks

Wed, Oct 10, 11:20 AM
zturner added a comment to D53086: [PDB] Fix flaky `variables-locations.test` after PR38857.

You didn't include it here, but I notice the test file just writes clang-cl /Zi. Should we be passing -m32 or -m64? Currently, the test just runs with whatever architecture happens to be set via the VS command prompt. The behavior here is different on x86 and x64 so perhaps it requires separate tests.

Wed, Oct 10, 11:15 AM · Restricted Project
zturner added a comment to D53086: [PDB] Fix flaky `variables-locations.test` after PR38857.

By the way, what do you think, how can we make LLDB support aligned stacks? As far as I know, similar alignment problems are reproducible on non-Windows too.

Wed, Oct 10, 11:14 AM · Restricted Project
zturner committed rLLDB344154: Create a SymbolFile plugin for cross-platform PDB access..
Create a SymbolFile plugin for cross-platform PDB access.
Wed, Oct 10, 9:41 AM
zturner committed rL344154: Create a SymbolFile plugin for cross-platform PDB access..
Create a SymbolFile plugin for cross-platform PDB access.
Wed, Oct 10, 9:41 AM
zturner closed D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Wed, Oct 10, 9:40 AM

Tue, Oct 9

zturner committed rL344095: [git-llvm] Fix some issues surrouding EOL conversion on Windows..
[git-llvm] Fix some issues surrouding EOL conversion on Windows.
Tue, Oct 9, 4:44 PM
zturner closed D51444: [git-llvm] Fix eol conversion on Windows for explicit CRLF files.
Tue, Oct 9, 4:44 PM
zturner updated the diff for D53002: Create a new symbol file plugin for cross-platform PDB symbolization.

Use lldbassert. The main reason I didn't use this originally is because it didn't used to be a hard assert. But since it seems like it is now, I've no problem standardizing on this.

Tue, Oct 9, 4:14 PM
zturner added inline comments to D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Tue, Oct 9, 3:57 PM
zturner updated the diff for D53002: Create a new symbol file plugin for cross-platform PDB symbolization.

Addressed suggestions from Leonard.

Tue, Oct 9, 3:30 PM
zturner added a comment to D53002: Create a new symbol file plugin for cross-platform PDB symbolization.

@lemo Thanks for the detailed review! I'll fix the suggestions and upload a new patch in a bit.

Tue, Oct 9, 3:09 PM
zturner committed rL344081: [PDB] Fix another bug in globals stream name lookup..
[PDB] Fix another bug in globals stream name lookup.
Tue, Oct 9, 2:23 PM
zturner added a comment to D45213: [COFF][LLD] Add link support for precompiled headers .objs.

BTW, sorry this took so long for anyone to review. I think partly it's because nobody here really taken the time to understand in detail how MS precompiled headers works, so we were all hoping someone else would review it. :-/

Tue, Oct 9, 2:18 PM
zturner added inline comments to D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Tue, Oct 9, 2:13 PM
zturner updated the diff for D53002: Create a new symbol file plugin for cross-platform PDB symbolization.

Addressed all issues pointed out by various people, and added a few more tests. There's only a limited amount of stuff we can do without a running process, so I'll probably have to resort to lldb-test soon to be able to print more detailed information.

Tue, Oct 9, 2:12 PM
zturner committed rL344066: Fix lld test..
Fix lld test.
Tue, Oct 9, 11:37 AM
zturner committed rLLD344066: Fix lld test..
Fix lld test.
Tue, Oct 9, 11:37 AM
zturner added a comment to D45213: [COFF][LLD] Add link support for precompiled headers .objs.

How big are the object files?

Tue, Oct 9, 11:22 AM
zturner committed rL344063: [PDB] Fix failure on big endian machines..
[PDB] Fix failure on big endian machines.
Tue, Oct 9, 11:01 AM
zturner added inline comments to D53021: lld-link: Use /pdbsourcepath: for more places when present..
Tue, Oct 9, 10:32 AM
zturner added inline comments to D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Tue, Oct 9, 10:23 AM
zturner added inline comments to D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Tue, Oct 9, 10:04 AM
zturner added a comment to rL343951: [PDB] Add the ability to lookup global symbols by name..

This seems to have caused a build bot break on all PPC big endian bots. Sample build:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/20094

There have been 50+ subsequent builds with the same failure. This should be pulled to bring the bots back to green. Keep in mind that this also keeps the s390x bots in the red as well (I think s390 is also a big endian machine).
I haven't taken a close look at the patch to see what might be causing the problems, but the reinterpret_cast there certainly seems like a very likely candidate.

Tue, Oct 9, 9:38 AM
zturner added a comment to rL343951: [PDB] Add the ability to lookup global symbols by name..

It was actually failing for a different reason first (malformed run line
due to backquote), which is the failure in the bot you linked. But I fixed
that and it never went green due to the new issue which you mentioned,
which does seem to be endianness related. Either way, it doesn't change
your point that the bots have been red for a long time. Do you have any
suggestions on how I can diagnose this? I don't have a big endian machine.

Tue, Oct 9, 7:35 AM

Mon, Oct 8

zturner committed rL344002: Remove unused variable..
Remove unused variable.
Mon, Oct 8, 3:58 PM
zturner updated the diff for D53002: Create a new symbol file plugin for cross-platform PDB symbolization.

Fix the test. I accidentally clang-formatted it which messed up the check lines. Turn clang-format off for this file.

Mon, Oct 8, 3:56 PM
zturner created D53002: Create a new symbol file plugin for cross-platform PDB symbolization.
Mon, Oct 8, 3:51 PM
zturner committed rL344001: [PDB] fix a bug in global stream name lookup..
[PDB] fix a bug in global stream name lookup.
Mon, Oct 8, 3:40 PM
zturner added a comment to D52998: [benchmark] Disable exceptions in Microsoft STL.

It's not enough to just set _HAS_EXCEPTIONS=1, it has to match whatever the value of /EH is passed to the compiler. So if we need to be able to throw catch exceptions, then you should pass /EHsc and not touch _HAS_EXCEPTIONS (technically you could set it to 1 but that's the default). And if we don't need to be able to throw or catch exceptions, then we pass /EHs-c- (which is what we do today) and also set _HAS_EXCEPTIONS=0. But the two should agree with each other.

Mon, Oct 8, 3:15 PM
zturner updated subscribers of D52998: [benchmark] Disable exceptions in Microsoft STL.

_HAS_EXCEPTIONS=0 is an undocumented STL specific thing that the library
implementation uses to mean "don't write code that does throw X;, do
something else instead".

Mon, Oct 8, 2:30 PM
zturner added a comment to D51444: [git-llvm] Fix eol conversion on Windows for explicit CRLF files.
In D51444#1258115, @rnk wrote:

lgtm

Python 3.7.0 32-bit

^ There's your problem =P

Mon, Oct 8, 1:11 PM
zturner committed rL343971: Don't use back-quotes in a run line..
Don't use back-quotes in a run line.
Mon, Oct 8, 8:15 AM

Sun, Oct 7

zturner committed rL343953: Fix a -Wsign-compare warning..
Fix a -Wsign-compare warning.
Sun, Oct 7, 9:46 PM
zturner committed rL343952: Fix a compilation failure on non-MSVC compilers..
Fix a compilation failure on non-MSVC compilers.
Sun, Oct 7, 9:37 PM
zturner updated the diff for D51444: [git-llvm] Fix eol conversion on Windows for explicit CRLF files.

I never got around to getting this committed, but incidentally I built a new machine over the weekend and ran into the same problem *plus* an additional problem which is that with Python 3, git apply doesn't like it when universal_newlines is set to true. Without this additional changes, it is impossible to push anything using the following configuration:

Sun, Oct 7, 9:26 PM
zturner committed rL343951: [PDB] Add the ability to lookup global symbols by name..
[PDB] Add the ability to lookup global symbols by name.
Sun, Oct 7, 9:22 PM

Thu, Oct 4

zturner added a comment to D52831: [lit] Only return a found bash executable on Windows if it can understand Windows paths.
execute_external && isWindows -> 'cmd.exe'
Thu, Oct 4, 3:51 PM
zturner added a comment to D52831: [lit] Only return a found bash executable on Windows if it can understand Windows paths.

To be clear, I never suggested we should treat cmd.exe as a valid choice of
external shell. What I meant was that the two supported configurations
should be:

Thu, Oct 4, 1:55 PM
zturner added a comment to D52773: clang-cl: Add /showFilenames option (PR31957).

I agree magic environment variables are bad, but without it we don’t
address the only current actual use we have for this, which is making the
vs integration print filenames. Detecting compiler version from inside the
integration is hard

We need some solution to this anyhow; e.g. say "this now requires clang 8.0", or have a clang version dropdown in the UI (which defaults to the latest release), or something. We can't add an env var for every future flag that the vs integration might want to use.

Thu, Oct 4, 11:30 AM
zturner updated subscribers of D52773: clang-cl: Add /showFilenames option (PR31957).

I agree magic environment variables are bad, but without it we don’t
address the only current actual use we have for this, which is making the
vs integration print filenames. Detecting compiler version from inside the
integration is hard, but with an environment variable it’s very easy to
solve.

Thu, Oct 4, 5:38 AM

Wed, Oct 3

zturner added inline comments to D52843: Update Clang Windows getting started docs.
Wed, Oct 3, 4:02 PM
zturner added inline comments to D52843: Update Clang Windows getting started docs.
Wed, Oct 3, 4:01 PM
zturner updated subscribers of D52831: [lit] Only return a found bash executable on Windows if it can understand Windows paths.

I personally think we should never use bash on Windows (wsl being the
exception but that won’t identify as Windows anyway). There’s value in
consistency and in my ideal world the lit shell would be feature rich
enough that we wouldn’t have to use bash *anywhere*. In any case right now
the configuration matrix is Windows (lit shell) and non Windows (bash). I
don’t think we should grow this by supporting Windows (bash)

Wed, Oct 3, 8:43 AM

Tue, Oct 2

zturner added a reviewer for D52800: Java import sorting in clang-format: krasimir.
Tue, Oct 2, 2:20 PM
zturner added inline comments to D52799: [llvm-pdbutil] Pretty print PDBSymbolUsingNamespace symbols.
Tue, Oct 2, 1:49 PM
zturner added inline comments to D52799: [llvm-pdbutil] Pretty print PDBSymbolUsingNamespace symbols.
Tue, Oct 2, 1:48 PM
zturner committed rC343629: [cl-compat] Change /JMC from unsupported to ignored..
[cl-compat] Change /JMC from unsupported to ignored.
Tue, Oct 2, 1:44 PM
zturner committed rL343629: [cl-compat] Change /JMC from unsupported to ignored..
[cl-compat] Change /JMC from unsupported to ignored.
Tue, Oct 2, 1:44 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Tue, Oct 2, 1:44 PM
zturner created D52798: [cl-compat] Change /JMC from unsupported to ignored..
Tue, Oct 2, 1:15 PM
zturner closed D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.
Tue, Oct 2, 12:44 PM · lld
zturner updated subscribers of D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

Ahh I see. You need to say "Differential Revision:
https://reviews.llvm.org/D50404" in the commit message. just the URL is
not sufficient to trigger the magic auto-close.

Tue, Oct 2, 12:10 PM · lld
zturner added a comment to D52618: [Windows] A basic implementation of memory allocations in a debuggee process.

One idea would be to define some lit substitutions like %debuginfo. It’s
true you can produce a gcc style command line that will be equivalent to a
clang-cl invocation but it won’t be easy. eg you’ll needing to pass
-fms-compatibility as well as various -I for includes.

It may be easier to have substitutions instead

Using substitutions SGTM.

I am not sure if this is a good idea, but it had occured to me that we could put -fms-compatibility and friends into a substitution of it's own, which would be computed by lit (through some equivalent of clang++ -### ?). That way, the tests could still use g++ syntax, only the command lines would contain an extra %cflags argument. This has the advantage of extra flexibility over a predefined set of compile commands (%compile_with_debug, %compile_without_debug, ...), and it might be sufficient to make cross-compiling work, if we ever need it.

Tue, Oct 2, 12:00 PM · Restricted Project
zturner added a comment to D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}.

It would be great if we could land this soon-ish. I have a lot of people complaining that lld-link rejects /DEBUG:FASTLINK when using the VS extension. Is there any work left to be done or can we land this?

Tue, Oct 2, 11:42 AM · lld
zturner added inline comments to D52773: clang-cl: Add /showFilenames option (PR31957).
Tue, Oct 2, 11:40 AM