Merge branch 'master' into '6.0/stage' #18
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Every few builds or so, a vmtest VM crashes after printing "x86: Booting SMP configuration:". After some difficult debugging, I determined that the crash happens in arch/x86/realmode/rm/trampoline_64.S (the code that initializes secondary CPUs) at the ljmp from startup_32 to startup_64. The real problem happens earlier in startup_32: movl $pa_trampoline_pgd, %eax movl %eax, %cr3 Sometimes, the store to CR3 "fails" and CR3 remains zero, which causes the later ljmp to triple fault. This can be reproduced by the following script: #!/bin/sh curl -L 'https://www.dropbox.com/sh/2mcf2xvg319qdaw/AABFKsISWRpndNZ1gz60O-qSa/x86_64/vmlinuz-5.8.0-rc7-vmtest1?dl=1' -o vmlinuz cat > commands.gdb << "EOF" set confirm off target remote :1234 # arch/x86/realmode/rm/trampoline_64.S:startup_32 after CR3 store. hbreak *0x9ae09 if $cr3 == 0 command info registers eax cr3 quit 1 end # kernel/smp.c:smp_init() after all CPUs have been brought up. If we get here, # the bug wasn't triggered. hbreak *0xffffffff81ed4484 command kill quit 0 end continue EOF while true; do qemu-system-x86_64 -cpu host -enable-kvm -smp 64 -m 128M \ -nodefaults -display none -serial file:/dev/stdout -no-reboot \ -kernel vmlinuz -append 'console=0,115200 panic=-1 nokaslr' \ -s -S & gdb -batch -x commands.gdb || exit 1 done This seems to be a problem with nested virtualization that was fixed by Linux kernel commit b4d185175bc1 ("KVM: VMX: give unrestricted guest full control of CR3") (in v4.17). Apparently, the Google Cloud hosts that Travis runs on are missing this fix. We obviously can't patch those hosts, but we can work around it. Disabling unrestricted guest support in the Travis VM causes CR3 stores in the nested vmtest VM to be emulated, bypassing the bug. Signed-off-by: Omar Sandoval <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
The __init_subclass__ and __class_getitem__ methods are always class methods even if not decorated as such, so format them accordingly. Signed-off-by: Omar Sandoval <[email protected]>
Lots if interfaces in drgn transparently turn an integer Object into an int by using __index__(), so add an IntegerLike protocol for this and use it everywhere applicable. Signed-off-by: Omar Sandoval <[email protected]>
Rather than duplicating Union[str, bytes, os.PathLike] everywhere, add an alias. Also make it explicitly os.PathLike[str] or os.PathLike[bytes] to get rid of some mypy --strict errors. Signed-off-by: Omar Sandoval <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
This will be used to support relative imports. Signed-off-by: Omar Sandoval <[email protected]>
Mainly for completeness, as I don't really like using them in my own projects. Signed-off-by: Omar Sandoval <[email protected]>
The helpers implemented in C have Python wrappers only for the purpose of documentation. This is because drgndoc ignores all imports when recursively documenting attributes. However, mypy uses the convention that aliased imports (i.e., import ... as ... or from ... import ... as ...) are considered re-exported, so we can follow that convention and include aliased imports. (mypy also considered attributes in __all__ as re-exported, so we should probably follow that in the future, too, but for now aliased imports are enough). This lets us get rid of the Python wrappers. Signed-off-by: Omar Sandoval <[email protected]>
We can get rid of the :include: and :exclude: options by deciding solely based on whether a node has a docstring. Empty docstrings can be used to indicate nodes that should be included with no additional content. The __init__() method must now also have a docstring in order to be documented. Additionally, the directives are now fully formatted by the Formatter rather than being split between the Formatter and DrgnDocDirective. Signed-off-by: Omar Sandoval <[email protected]>
One of the blockers for adding type annotations to helpers is that some helpers need to be overloaded, but drgndoc doesn't support that. This adds support. Each function now tracks all of its overloaded signature, each of which may be documented separately. The formatted output (for functions/methods and classes with __init__()) combines all of the documented overloads. Signed-off-by: Omar Sandoval <[email protected]>
Now that drgndoc can handle overloads and we have the IntegerLike and Path aliases, we can add type annotations to all helpers. There are also a couple of functional changes that snuck in here to make annotating easier. Signed-off-by: Omar Sandoval <[email protected]>
The remaining warnings are all no-any-return, which is hard to avoid in drgn. Signed-off-by: Omar Sandoval <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
As of QEMU commit a5804fcf7b22 ("9pfs: local: ignore O_NOATIME if we
don't have permissions") (in v5.1.0), QEMU handles O_NOATIME sanely, so
we don't need the LD_PRELOAD hack. Since we're adding a version check,
make the multidevs check based on the version, too.
Signed-off-by: Omar Sandoval <[email protected]>
This picks up a newer version of QEMU and lets us use udevadm trigger -w. Let's also explicitly add "os: linux" to silence the config validation. Signed-off-by: Omar Sandoval <[email protected]>
drgn_object_init() is available in drgh.h file and seems to a required call before calling drgn_program_find_object(). Without this, trying to call drgn_object_init() from an external C application results in undefined reference. Signed-off-by: Aditya Sarwade <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
It's still useful to have an escape hatch for names we don't want documented. Signed-off-by: Omar Sandoval <[email protected]>
E.g., drgndoc:: foo.bar() should emit py:method:: foo.bar() regardless of a previous py:module directive. Signed-off-by: Omar Sandoval <[email protected]>
Program::objects is used to store references to objects that must stay alive while the Program is alive. It is currently a PyDict where the keys are the object addresses as PyLong and the values are the objects themselves. This has two problems: 1. Allocating the key as a full object is obviously wasteful. 2. PyDict doesn't have an API for reserving capacity ahead of time, which we want for an upcoming change. Both of these are easily fixed by using our own hash table. Signed-off-by: Omar Sandoval <[email protected]>
…_bit}() Most places that call these check has_platform and return an error, and those that don't can live with the extra check. Signed-off-by: Omar Sandoval <[email protected]>
struct drgn_program has a bunch of state scattered around. Group it together more logically, even if it means sacrificing some padding here and there. Signed-off-by: Omar Sandoval <[email protected]>
This is preparation for associating types with a program. Signed-off-by: Omar Sandoval <[email protected]>
sdimitro
approved these changes
Aug 31, 2020
…eader If we fail to read an include directory in read_file_name_table(), we need to free the directory hashes. Signed-off-by: Omar Sandoval <[email protected]>
This is more clear: although these flags happen to be encoded with the DWARF tag, they are flags regarding the DIE. Signed-off-by: Omar Sandoval <[email protected]>
The CU unit length and DIE offset are both limited by the size of the mapped debugging information, i.e., size_t. Signed-off-by: Omar Sandoval <[email protected]>
As a small simplification, we can take commit 9bb2cce ("Enable DWARF indexing to work with partial units") further and not look at the tag of the top-level DIE at all. Signed-off-by: Omar Sandoval <[email protected]>
We currently assume that if DW_AT_declaration is present, it is true. This seems to be true in practice, and I see no reason to ever use DW_FORM_flag with a value of zero. There's no performance hit to handle it, though, so we might as well. Signed-off-by: Omar Sandoval <[email protected]>
In read_cus(), the master thread can use the final CUs vector directly and the rest of the threads can merge their private vectors in. This consistently shaves a few milliseconds off of startup. Signed-off-by: Omar Sandoval <[email protected]>
I originally copied the sections into each compilation unit to avoid a pointer indirection, but performance-wise it's a wash, so we might as well save the memory. This will be more important when we keep the CUs after indexing. Signed-off-by: Omar Sandoval <[email protected]>
It's very unlikely that we'll ever index more than 4 billion DIEs in a single shard, so we can shrink the index a bit by using uint32_t indices (and uint8_t tag). Signed-off-by: Omar Sandoval <[email protected]>
This is preparation for the next change where we'll need to do two passes over the CUs. Signed-off-by: Omar Sandoval <[email protected]>
We currently handle DIEs with a DW_AT_specification attribute by parsing
the corresponding declaration to get the name and inserting the DIE as
usual. This has a couple of problems:
1. It only works if DW_AT_specification refers to the same compilation
unit, which is true for DW_FORM_ref{1,2,4,8,_udata}, but not
DW_FORM_ref_addr. As a result, drgn doesn't support the latter.
2. It assumes that the DIE with DW_AT_specification is in the correct
"scope". Unfortunately, this is not true for g++: for a variable
definition in a C++ namespace, it generates a DIE with
DW_AT_declaration as a child of the DW_TAG_namespace DIE and a DIE
which refers to the declaration with DW_AT_specification _outside_ of
the DW_TAG_namespace as a child of the DW_TAG_compilation_unit DIE.
Supporting both of these cases requires reworking how we handle
DW_AT_specification. This commit takes an approach of parsing the DWARF
data in two passes: the first pass reads the abbrevation and file name
tables and builds a map of instances of DW_AT_specification; the second
pass indexes DIEs as before, but ignores DIEs with DW_AT_specification
and handles DIEs with DW_AT_declaration by looking them up in the map
built by the first pass.
This approach is a 10-20% regression in indexing time in the benchmarks
I ran. Thankfully, it is not 100% slower for a couple of reasons. The
first is that the two passes are simpler than the original combined
pass. The second is that a decent part of the indexing time is spent
faulting in the mapped debugging information, which only needs to happen
once (even if the file is cached, minor page faults add non-negligible
overhead).
This doesn't handle DW_AT_specification "chains" yet, but neither did
the original code. If it is necessary, it shouldn't be too difficult to
add.
Signed-off-by: Omar Sandoval <[email protected]>
Now that we can handle a DW_AT_specification that references another compilation unit, add support for DW_FORM_ref_addr. Signed-off-by: Omar Sandoval <[email protected]>
There are a couple of related ways that we can cause undefined behavior when parsing a malformed DWARF or depmod index file: 1. There are several places where we increment the cursor to skip past some data. It is undefined behavior if the result points out of bounds of the data, even if we don't attempt to dereference it. 2. read_in_bounds() checks that ptr <= end. This pointer comparison is only defined if ptr and end both point to elements of the same array object or one past the last element. If ptr has gone past end, then this comparison is likely undefined anyways. Fix it by adding a helper to skip past data with bounds checking. Then, all of the helpers can assume that ptr <= end and maintain that invariant. while we're here and auditing all of the call sites, let's clean up the API and rename it from read_foo() to the less generic mread_foo(). Signed-off-by: Omar Sandoval <[email protected]>
This is needed for a future change where we'll want to save an error and return it multiple times. Signed-off-by: Omar Sandoval <[email protected]>
…iterator_next() For namespace support, we will want to access the struct drgn_dwarf_index_die for namespaces instead of the Dwarf_Die. Split drgn_dwarf_index_get_die() out of drgn_dwarf_index_iterator_next(). Signed-off-by: Omar Sandoval <[email protected]>
In order to index namespaces lazily, we need the CU structures. Rename struct compilation_unit to the less generic struct drgn_dwarf_index_cu and keep the CUs in a vector in the dindex. Signed-off-by: Jay Kamat <[email protected]>
DWARF represents namespaces with DW_TAG_namespace DIEs. Add these to the DWARF index, with each namespace being its own sub-index. We only index the namespace itself when it is first accessed, which should help with startup time and simplifies tracking. Signed-off-by: Jay Kamat <[email protected]>
The current name is too verbose. Let's go with a shorter, more generic name. Signed-off-by: Omar Sandoval <[email protected]>
Debugging information tracking is currently in two places: drgn_program finds debugging information, and drgn_dwarf_index stores it. Both of these responsibilities make more sense as part of drgn_debug_info, so let's move them there. This prepares us to track extra debugging information that isn't pertinent to indexing. This also reworks a couple of details of loading debugging information: - drgn_dwarf_module and drgn_dwfl_module_userdata are consolidated into a single structure, drgn_debug_info_module. - The first pass of DWARF indexing now happens in parallel with reading compilation units (by using OpenMP tasks). Signed-off-by: Omar Sandoval <[email protected]>
If we create a pending CU for a namespace, then add more CUs to the index, the CU might get reallocated, resulting in a use after free. Fix it by storing the index of the CU instead of the pointer. Signed-off-by: Omar Sandoval <[email protected]>
Signed-off-by: Omar Sandoval <[email protected]>
The elfutils header files should be treated as if they were in the standard location, so use -isystem instead of -I. Signed-off-by: Omar Sandoval <[email protected]>
I recently hit a couple of CI failures caused by relying on transitive includes that weren't always present. include-what-you-use is a Clang-based tool that helps with this. It's a bit finicky and noisy, so this adds scripts/iwyu.py to make running it more convenient (but not reliable enough to automate it in Travis). This cleans up all reasonable include-what-you-use warnings and reorganizes a few header files. Signed-off-by: Omar Sandoval <[email protected]>
The fix was backported to QEMU's 5.0 stable branch and released in 5.0.1. Signed-off-by: Omar Sandoval <[email protected]>
prakashsurya
approved these changes
Sep 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.