Page MenuHomePhabricator

simon.giesecke (Simon Giesecke)
User

Projects

User does not belong to any projects.

User Details

User Since
May 7 2021, 4:15 AM (82 w, 5 d)

Recent Activity

Oct 10 2022

simon.giesecke committed rG2f46f5090737: Add llvm-gsymutil to the Bazel build files. (authored by simon.giesecke).
Add llvm-gsymutil to the Bazel build files.
Oct 10 2022, 4:12 AM · Restricted Project
simon.giesecke closed D135568: [Bazel] Add llvm-gsymutil to the Bazel build files..
Oct 10 2022, 4:12 AM · Restricted Project, Restricted Project, Restricted Project
simon.giesecke added a project to D135568: [Bazel] Add llvm-gsymutil to the Bazel build files.: Restricted Project.
Oct 10 2022, 3:43 AM · Restricted Project, Restricted Project, Restricted Project
simon.giesecke retitled D135568: [Bazel] Add llvm-gsymutil to the Bazel build files. from Add llvm-gsymutil to the Bazel build files. to [Bazel] Add llvm-gsymutil to the Bazel build files..
Oct 10 2022, 3:43 AM · Restricted Project, Restricted Project, Restricted Project
simon.giesecke added reviewers for D135568: [Bazel] Add llvm-gsymutil to the Bazel build files.: clayborg, GMNGeoffrey.
Oct 10 2022, 3:19 AM · Restricted Project, Restricted Project, Restricted Project
simon.giesecke requested review of D135568: [Bazel] Add llvm-gsymutil to the Bazel build files..
Oct 10 2022, 3:16 AM · Restricted Project, Restricted Project, Restricted Project

Aug 31 2022

simon.giesecke accepted D132912: [llvm-gsymutil] Fix tracking of currently open file.
Aug 31 2022, 3:33 AM · Restricted Project, Restricted Project
simon.giesecke added a comment to D132912: [llvm-gsymutil] Fix tracking of currently open file.

@vikmik Thanks a lot for fixing this!

Aug 31 2022, 3:33 AM · Restricted Project, Restricted Project

Feb 4 2022

simon.giesecke added inline comments to D118927: [clang-tidy] Fix invalid fix-it for cppcoreguidelines-prefer-member-initializer.
Feb 4 2022, 12:36 AM · Restricted Project, Restricted Project

Dec 9 2021

simon.giesecke added inline comments to D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods.
Dec 9 2021, 1:54 AM · Restricted Project, Restricted Project
simon.giesecke added a comment to D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods.

Thanks a lot for addressing this issue! I am just trying it on our codebase.

The problem actually has nothing to do with the out-of-line definition being inline; the problem is that hasDeclaration() of the memberExpr() will match the out-of-line definition, which obviously isn't marked static, so isStaticStorageClass() won't match.

Hm, an out-of-line definition *cannot* have the static keyword. I wonder if it's actually a bug (in the AST? or just the matcher?) that isStaticStorageClass doesn't match here? I guess that other checks that use isStaticStorageClass might be affected by this too?

isStaticStorageClass() calls through to FunctionDecl::getStorageClass() which reports the storage as written on that declaration in source. So this isn't a bug in the AST, it's a more of a misunderstanding of whether we're querying the storage duration/linkage for the method or whether we're querying whether it was written with the static keyword.

Dec 9 2021, 1:49 AM · Restricted Project, Restricted Project

Dec 6 2021

simon.giesecke added a comment to D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods.

Thanks a lot for addressing this issue! I am just trying it on our codebase.

Dec 6 2021, 2:36 AM · Restricted Project, Restricted Project

Nov 9 2021

simon.giesecke accepted D113429: [clang-tidy] Use `hasCanonicalType()` matcher in `bugprone-unused-raii` check.

LGTM, thanks a lot!

Nov 9 2021, 2:18 AM · Restricted Project

Oct 7 2021

simon.giesecke added a comment to D93543: clang-tidy: Add `-use-color` for `run-clang-tidy.py` in order to make it sane with `clang-tidy` coloring argument.

I think this would be a useful change. In the current state, one needs to modify the script to run this in a context where coloring is not supported.

Oct 7 2021, 5:44 AM · Restricted Project, Restricted Project

Sep 30 2021

simon.giesecke added inline comments to D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning.
Sep 30 2021, 1:08 AM · Restricted Project, Restricted Project
simon.giesecke added inline comments to D69764: [clang-format] Add Left/Right Const fixer capability.
Sep 30 2021, 1:00 AM · Restricted Project, Restricted Project

Sep 29 2021

simon.giesecke added inline comments to D69764: [clang-format] Add Left/Right Const fixer capability.
Sep 29 2021, 9:01 AM · Restricted Project, Restricted Project

Sep 6 2021

simon.giesecke added inline comments to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..
Sep 6 2021, 6:12 AM · Restricted Project, debug-info

Jul 23 2021

simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

If you're adding command-line options a) they might better belong in separate later patches, and b) the documentation should be updated to reference them.

a) Hm, ok, this will just leave us without without the option to disable GSYM usage after the first patch.

Yes, this is true. I don't know if that's an issue or not though? What's the impact on a typical end user in that interim state? In other words, is it likely that something bad will happen?

Jul 23 2021, 3:02 AM · debug-info, Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

@jhenderson I am struggling with even building cross-project-tests. How do I enable those? I tried setting calling cmake with either -DLLVM_TOOL_CROSS_PROJECT_TESTS_BUILD=TRUE or -DLLVM_ENABLE_PROJECTS="clang;cross-platform-tests", but those don't seem to do the job.

It's "cross-project-tests" not "cross-platform-tests" in the LLVM_ENABLE_PROJECTS listing.

Jul 23 2021, 2:31 AM · debug-info, Restricted Project

Jul 22 2021

simon.giesecke added a project to D105985: Support GSYM in llvm-symbolizer.: debug-info.
Jul 22 2021, 4:19 AM · debug-info, Restricted Project

Jul 21 2021

simon.giesecke updated the diff for D105985: Support GSYM in llvm-symbolizer..

Add command line options to control GSYM behaviour.

Jul 21 2021, 8:32 AM · debug-info, Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

@jhenderson I am struggling with even building cross-project-tests. How do I enable those? I tried setting calling cmake with either -DLLVM_TOOL_CROSS_PROJECT_TESTS_BUILD=TRUE or -DLLVM_ENABLE_PROJECTS="clang;cross-platform-tests", but those don't seem to do the job.

Jul 21 2021, 4:51 AM · debug-info, Restricted Project

Jul 19 2021

simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

A better approach might be to write the GSYM test from scratch, as if the DWARF test didn't exist, and then write a DWARF equivalent one (alternatively the other way around would also work). The important parts of sym.test could then be split off.

Jul 19 2021, 12:43 AM · debug-info, Restricted Project

Jul 16 2021

simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..
  1. Make sure to review the llvm-symbolizer documentation to see if there's anything there that needs adding/updating given this change (there may not be).

I checked llvm/docs/CommandGuide/llvm-symbolizer.rst, and it doesn't say anything right now about how the data source is selected. Maybe it should, and I can add something, but then this should probably be more than just saying that GSYM is preferred when it's present. I think there's similar logic for .dwp files?

I don't think we need to go into that in this change. I guess the point of the llvm-symbolizer documentation is more about how to use the tool, not what does the tool do under-the-hood, but I thought it was worth checking.

Jul 16 2021, 2:11 AM · debug-info, Restricted Project
simon.giesecke committed rGa12000e4289b: Reformat files. (authored by simon.giesecke).
Reformat files.
Jul 16 2021, 12:40 AM
simon.giesecke closed D105982: Reformat files..
Jul 16 2021, 12:39 AM · Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..
  1. Make sure to review the llvm-symbolizer documentation to see if there's anything there that needs adding/updating given this change (there may not be).
Jul 16 2021, 12:35 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D105985: Support GSYM in llvm-symbolizer..

Added top-level comment to sym-gsymonly.test

Jul 16 2021, 12:31 AM · debug-info, Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

@clayborg Could you comment on the TODOs referring to llvm-symbolizer command line options:

// TODO We should provide an option to provide an alternative directory for

// GSYM files.

and

// TODO There should be an option that disables the preference for GSYM.
Jul 16 2021, 12:29 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D105985: Support GSYM in llvm-symbolizer..

Address clayborg's comments, implemented getLineInfoForAddressRange.

Jul 16 2021, 12:27 AM · debug-info, Restricted Project
simon.giesecke added inline comments to D105985: Support GSYM in llvm-symbolizer..
Jul 16 2021, 12:27 AM · debug-info, Restricted Project

Jul 15 2021

simon.giesecke added inline comments to D105985: Support GSYM in llvm-symbolizer..
Jul 15 2021, 2:31 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D105985: Support GSYM in llvm-symbolizer..

Addressed some comments by clayborg.

Jul 15 2021, 2:30 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D105982: Reformat files..

Rebase to current main.

Jul 15 2021, 2:29 AM · Restricted Project
simon.giesecke added inline comments to D105985: Support GSYM in llvm-symbolizer..
Jul 15 2021, 1:38 AM · debug-info, Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

I have zero experience with or knowledge about gsym, so I can't comment on the implementation details, but some higher-level comments:

  1. Make sure to review the llvm-symbolizer documentation to see if there's anything there that needs adding/updating given this change (there may not be).
Jul 15 2021, 1:08 AM · debug-info, Restricted Project

Jul 14 2021

simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

Note that sym-gsymonly.test is mostly a copy of sym.test & using the different input file. The only other difference is that I removed the column number for main, which we don't have available from the GSYM file, IIUC.

Jul 14 2021, 8:00 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D105985: Support GSYM in llvm-symbolizer..

Resolved duplication between getLineInfoForAddress and getInliningInfoForAddress.

Jul 14 2021, 7:54 AM · debug-info, Restricted Project
simon.giesecke added a comment to D105985: Support GSYM in llvm-symbolizer..

There are several TODO markers in the code. Please provide some feedback on those.

Jul 14 2021, 7:43 AM · debug-info, Restricted Project
simon.giesecke requested review of D105985: Support GSYM in llvm-symbolizer..
Jul 14 2021, 7:39 AM · debug-info, Restricted Project
simon.giesecke requested review of D105982: Reformat files..
Jul 14 2021, 7:36 AM · Restricted Project
simon.giesecke added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

That makes me wonder whether the direction this takes is actually the right one. Another option might be to add support for GSYM to llvm-symbolizer. Do you think this would be a better direction?

It would be great to add support for GSYM into llvm-symbolizer! It also should be very easy to add if users supply the symbol file to llvm-symbolizer as it would be easy to sniff the first few bytes of the files for magic bytes and routing to the correctly file format's parser.

I am working on this now :) I am trying to find my way around llvm-symbolizer, and I came across DILineInfo, which is documented as "A format-neutral container for source line information." Is there already a way to get a DILineInfo from GSYM files? I haven't found any direct way to do this, in GsymReader or so.

Not yet! Feel free to add an accessor to convert any GSYM data structures into DI data structures. There are two main formats that come out of GSYM: the full llvm::gsym::FunctionInfo or the llvm::gsym::LookupResult. llvm::gsym::FunctionInfo is the complete information for the function with the name, address range, line table and inline info. This info covers all addresses in the function. llvm::gsym::LookupResult is just the information you need for a single address and in a much friendlier format where all strings have been retrieved from the string table. It should be very easy to make any needed conversions. Let me know if you have any questions.

Jul 14 2021, 1:19 AM · Restricted Project, debug-info

Jul 13 2021

simon.giesecke added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

That makes me wonder whether the direction this takes is actually the right one. Another option might be to add support for GSYM to llvm-symbolizer. Do you think this would be a better direction?

It would be great to add support for GSYM into llvm-symbolizer! It also should be very easy to add if users supply the symbol file to llvm-symbolizer as it would be easy to sniff the first few bytes of the files for magic bytes and routing to the correctly file format's parser.

Jul 13 2021, 11:08 AM · Restricted Project, debug-info

May 27 2021

simon.giesecke committed rG5f2d4b23b4c2: Add --quiet option to llvm-gsymutil to suppress output of warnings. (authored by simon.giesecke).
Add --quiet option to llvm-gsymutil to suppress output of warnings.
May 27 2021, 5:39 AM
simon.giesecke closed D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..
May 27 2021, 5:39 AM · Restricted Project
simon.giesecke added a comment to D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

I will need to add tests for these warnings since they don't already exist. These kinds of tests require a deep knowledge of making DWARF that contains each of these warnings, so I won't have you do this.

May 27 2021, 5:08 AM · Restricted Project

May 25 2021

simon.giesecke added a comment to D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

@clayborg I am not sure what it means you now accepted the revision? I am still not sure what to do about the tests.

May 25 2021, 11:59 PM · Restricted Project

May 21 2021

simon.giesecke added a comment to D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

Contents looks good, just need a test to ensure that warnings are quieted. There should be a test for each kind of warning, so it should be easy to run that same command with the --quiet option and verify that the warning does _not_ appear.

May 21 2021, 1:54 AM · Restricted Project

May 20 2021

simon.giesecke updated the diff for D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

Updated patch description with numbers from perf profile.

May 20 2021, 8:09 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

Fix cmdline.test

May 20 2021, 5:41 AM · Restricted Project
simon.giesecke updated the diff for D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..

Rebase patch on main

May 20 2021, 1:16 AM · Restricted Project
simon.giesecke requested review of D102829: Add --quiet option to llvm-gsymutil to suppress output of warnings..
May 20 2021, 1:06 AM · Restricted Project

May 19 2021

simon.giesecke committed rG0ddc75fd0834: Add option to llvm-gsymutil to read addresses from stdin. (authored by simon.giesecke).
Add option to llvm-gsymutil to read addresses from stdin.
May 19 2021, 11:12 PM
simon.giesecke closed D102224: Add option to llvm-gsymutil to read addresses from stdin..
May 19 2021, 11:11 PM · Restricted Project, debug-info
simon.giesecke updated the diff for D102224: Add option to llvm-gsymutil to read addresses from stdin..

Fix path handling in test case on Windows

May 19 2021, 6:39 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102219: Optimize GSymCreator::finalize..

FWIW, and unrelated to this patch: processing other, more complex gcc-built binaries, e.g. clang, produces a lot of the other warnings (with and without the patch). Not sure if that's concerning.

This is par for the course. Everyone produces debug info and the debug info usually has some issues. llvm-gsymutil is merely pointing out issues it runs into when parsing this information. llvm-gsymutil is also smart enough to ignore debug info that should have been stripped, but can't. This stems from the fact that linkers don't know anything about DWARF and they like to just concatenate all DWARF sections from each .o file and relocate any addresses that were not stripped. This means for debug info that had no relocations will still remain in the DWARF. For those functions, they typically get their addresses set to zero or -1. llvm-gsymutil knows to ignore these when it calls GsymCreator::SetValidTextRanges(...) after it parses the sections of the object file. Any function infos whose start address doesn't start in a valid text range will be omitted.

You can also build "llvm-dwarfdump" and then run it using the "--verify" option. It will point out any issues that exist in the DWARF.

So warnings are normal because there are many issues with the DWARF produced by all compilers and linkers. Some are worse than others, but in general we try to work around these issues and still produce a GSYM. Link Time Optimizations (LTO) also tends to outline many functions and also combine functions with common code, so you can have multiple DWARF functions point to the same address. If the line entries are all the same and the function name matches, that is easily detected, but sometimes we have templated code that is inlined, like std::shared_ptr::~shared_ptr<T>() that ends up creating the same code for multiple different instances and they get combined even though the line tables would look different.

May 19 2021, 4:30 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102224: Add option to llvm-gsymutil to read addresses from stdin..

@clayborg I addressed your comments now. Adding support for --json is something I will look into separately, if that's fine for you.

May 19 2021, 4:02 AM · Restricted Project, debug-info
simon.giesecke added inline comments to D102224: Add option to llvm-gsymutil to read addresses from stdin..
May 19 2021, 4:01 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102224: Add option to llvm-gsymutil to read addresses from stdin..

Addressed review comments

May 19 2021, 4:01 AM · Restricted Project, debug-info
simon.giesecke abandoned D102758: Add option to llvm-gsymutil to read addresses from stdin..
May 19 2021, 3:59 AM · Restricted Project
simon.giesecke requested review of D102758: Add option to llvm-gsymutil to read addresses from stdin..
May 19 2021, 3:58 AM · Restricted Project
simon.giesecke committed rG81b2fcf26fca: Use a non-recursive mutex in GsymCreator. (authored by simon.giesecke).
Use a non-recursive mutex in GsymCreator.
May 19 2021, 3:33 AM
simon.giesecke committed rG4ea4d9c066b6: Move FunctionInfo in addFunctionInfo rather than copying. (authored by simon.giesecke).
Move FunctionInfo in addFunctionInfo rather than copying.
May 19 2021, 3:33 AM
simon.giesecke committed rGf29c4c60978c: Avoid calculating the string hash twice in GsymCreator::insertString. (authored by simon.giesecke).
Avoid calculating the string hash twice in GsymCreator::insertString.
May 19 2021, 3:33 AM
simon.giesecke committed rGe102fd50f9c6: Reformat GSYMCreator.cpp (authored by simon.giesecke).
Reformat GSYMCreator.cpp
May 19 2021, 3:33 AM
simon.giesecke closed D102486: Use a non-recursive mutex in GsymCreator..
May 19 2021, 3:33 AM · debug-info, Restricted Project
simon.giesecke closed D102485: Move FunctionInfo in addFunctionInfo rather than copying..
May 19 2021, 3:33 AM · Restricted Project, debug-info
simon.giesecke closed D102484: Avoid calculating the string hash twice in GsymCreator::insertString..
May 19 2021, 3:33 AM · Restricted Project, debug-info
simon.giesecke closed D102483: Reformat GSYMCreator.cpp.
May 19 2021, 3:32 AM · debug-info, Restricted Project

May 18 2021

simon.giesecke added a comment to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

What's your use case such that this performance concern has come up for you?

May 18 2021, 11:28 PM · Restricted Project, debug-info
simon.giesecke updated the diff for D102224: Add option to llvm-gsymutil to read addresses from stdin..

Update using arc

May 18 2021, 4:37 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

Update using arc

May 18 2021, 3:38 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

Update using arc

May 18 2021, 3:35 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

You should probably get Phabricator working: https://llvm.org/docs/Phabricator.html

This will ensure your patches have context. If you submitting patches manually, you need to specify more context lines:

git diff -U999999

May 18 2021, 3:34 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102486: Use a non-recursive mutex in GsymCreator..

Update using arc

May 18 2021, 3:24 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D102485: Move FunctionInfo in addFunctionInfo rather than copying..

Update using arc

May 18 2021, 3:24 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102484: Avoid calculating the string hash twice in GsymCreator::insertString..

Update using arc

May 18 2021, 3:23 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102483: Reformat GSYMCreator.cpp.

Update using arc

May 18 2021, 3:19 AM · debug-info, Restricted Project
simon.giesecke abandoned D102674: Avoid calculating the string hash twice in GsymCreator::insertString..
May 18 2021, 2:00 AM · Restricted Project
simon.giesecke requested review of D102674: Avoid calculating the string hash twice in GsymCreator::insertString..
May 18 2021, 2:00 AM · Restricted Project

May 17 2021

simon.giesecke updated the summary of D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..
May 17 2021, 9:15 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..

One thing I don't know here is whether all relevant uses of DWARFUnit will end up iterating the children. If that's not the case, then this be done only optionally. We can't do it on-demand on the first call to getLastChild easily though, because then, e.g. GsymCreator, would need to synchronize accesses from multiple threads. But maybe this could be passed as an option to tryExtractDIEsIfNeeded?

May 17 2021, 9:14 AM · Restricted Project, debug-info
simon.giesecke requested review of D102634: Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded..
May 17 2021, 9:12 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

@clayborg Can you land the four patches again for me? Thanks!

May 17 2021, 12:28 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

@clayborg Can you land the four patches again for me? Thanks!

May 17 2021, 12:26 AM · debug-info, Restricted Project
simon.giesecke updated the diff for D102486: Use a non-recursive mutex in GsymCreator..
May 17 2021, 12:26 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102484: Avoid calculating the string hash twice in GsymCreator::insertString..

I moved the hash calculation out of the lock now, as suggested in D102486.

May 17 2021, 12:25 AM · Restricted Project, debug-info
simon.giesecke updated the diff for D102484: Avoid calculating the string hash twice in GsymCreator::insertString..
May 17 2021, 12:24 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102483: Reformat GSYMCreator.cpp.
May 17 2021, 12:19 AM · debug-info, Restricted Project

May 15 2021

simon.giesecke added a comment to D102487: Avoid underestimating the number of DIEs for a given debug info size..

After thinking about this again, I wonder:

  • How bad it would be to overestimate the number of entries; there might obviously be cases where this would lead to memory exhaustion. Not sure if we he have some standard means of recognizing a memory pressure situation in the LLVM codebase?
  • As @dblaikie asked If this may vary significantly depending on the platform, compiler, optimization level and other properties, and we should better probe this somehow for a given binary.
May 15 2021, 2:49 AM · Restricted Project, debug-info
simon.giesecke added a comment to D102483: Reformat GSYMCreator.cpp.

Was this all fixed by the auto linter?

May 15 2021, 2:45 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue

Could perhaps start by moving the CachedHashStringRef construction outside the lock?

The "Copy" argument is usually set to "false" so it won't end up copying the string. We only have to back the string up if the DWARF information had no mangled name for an entry and we and up calculating a fully qualified name by traversing the DWARF. This is rare though.

May 15 2021, 2:44 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue here. I was wondering if we could avoid locking the mutex here, but using thread-local tables until finalisation does not seem to be feasible since any thread might insert the same string IIUC and we want to have unique IDs at this point. A lock-free map might be an alternative here, though?

I am open to ideas to speed things up, but many things rely on strings being correctly uniqued: strings for function names and inlined function names, files in the file tables (which consist of two string: directory + basename).

May 15 2021, 2:43 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue

Could perhaps start by moving the CachedHashStringRef construction outside the lock?

May 15 2021, 2:41 AM · debug-info, Restricted Project

May 14 2021

simon.giesecke added a comment to D102486: Use a non-recursive mutex in GsymCreator..

GSYM conversion still spends a significant time for locking in GsymCreator::insertString even with only 8 threads, so lock contention seems to be generally an issue here. I was wondering if we could avoid locking the mutex here, but using thread-local tables until finalisation does not seem to be feasible since any thread might insert the same string IIUC and we want to have unique IDs at this point. A lock-free map might be an alternative here, though?

May 14 2021, 5:36 AM · debug-info, Restricted Project
simon.giesecke added a comment to D102487: Avoid underestimating the number of DIEs for a given debug info size..

I came across this when analyzing a perf profile of running llvm-gsymutil. I noticed that quite a lot of time was spent in resizing the DieArray, and then checking the actual number of entries relative to getDebugInfoSize() and found it to be rather in the range of 6-12 bytes (in most cases 6-8 actually). This was a RelWithDebInfo build using clang 12.

May 14 2021, 5:26 AM · Restricted Project, debug-info
simon.giesecke requested review of D102487: Avoid underestimating the number of DIEs for a given debug info size..
May 14 2021, 5:20 AM · Restricted Project, debug-info
simon.giesecke requested review of D102486: Use a non-recursive mutex in GsymCreator..
May 14 2021, 5:18 AM · debug-info, Restricted Project
simon.giesecke requested review of D102485: Move FunctionInfo in addFunctionInfo rather than copying..
May 14 2021, 5:17 AM · Restricted Project, debug-info