Skip to content

Commit

Permalink
Merged main:82ebd333a889d2372c8445dc3d5d527ec48537db into amd-gfx:0e8…
Browse files Browse the repository at this point in the history
…d0cecbbff

Local branch amd-gfx 0e8d0ce Merged main:438ad9f2bf25575c474313de4ad85a5da6f69e4c into amd-gfx:b82566cedfb0
Remote branch main 82ebd33 [LLDB][Minidumps] Read x64 registers as 64b and handle truncation in the file builder (llvm#106473)
  • Loading branch information
SC llvm team authored and SC llvm team committed Aug 29, 2024
2 parents 0e8d0ce + 82ebd33 commit c7b3b9d
Show file tree
Hide file tree
Showing 17 changed files with 1,025 additions and 112 deletions.
1 change: 1 addition & 0 deletions clang-tools-extra/clangd/CollectMacros.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI,
if (Loc.isInvalid() || Loc.isMacroID())
return;

assert(isInsideMainFile(Loc, SM));
auto Name = MacroNameTok.getIdentifierInfo()->getName();
Out.Names.insert(Name);
size_t Start = SM.getFileOffset(Loc);
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/clangd/CollectMacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ class CollectMainFileMacros : public PPCallbacks {

void SourceRangeSkipped(SourceRange R, SourceLocation EndifLoc) override;

// Called when the AST build is done to disable further recording
// of macros by this class. This is needed because some clang-tidy
// checks can trigger PP callbacks by calling directly into the
// preprocessor. Such calls are not interleaved with FileChanged()
// in the expected way, leading this class to erroneously process
// macros that are not in the main file.
void doneParse() { InMainFile = false; }

private:
void add(const Token &MacroNameTok, const MacroInfo *MI,
bool IsDefinition = false, bool InConditionalDirective = false);
Expand Down
8 changes: 7 additions & 1 deletion clang-tools-extra/clangd/ParsedAST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,9 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
Marks = Patch->marks();
}
auto &PP = Clang->getPreprocessor();
PP.addPPCallbacks(std::make_unique<CollectMainFileMacros>(PP, Macros));
auto MacroCollector = std::make_unique<CollectMainFileMacros>(PP, Macros);
auto *MacroCollectorPtr = MacroCollector.get(); // so we can call doneParse()
PP.addPPCallbacks(std::move(MacroCollector));

PP.addPPCallbacks(
collectPragmaMarksCallback(Clang->getSourceManager(), Marks));
Expand All @@ -709,6 +711,10 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
log("Execute() failed when building AST for {0}: {1}", MainInput.getFile(),
toString(std::move(Err)));

// Disable the macro collector for the remainder of this function, e.g.
// clang-tidy checkers.
MacroCollectorPtr->doneParse();

// We have to consume the tokens before running clang-tidy to avoid collecting
// tokens from running the preprocessor inside the checks (only
// modernize-use-trailing-return-type does that today).
Expand Down
17 changes: 17 additions & 0 deletions clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,23 @@ TEST(DiagnosticTest, ClangTidySelfContainedDiagsFormatting) {
withFix(equalToFix(ExpectedFix2))))));
}

TEST(DiagnosticsTest, ClangTidyCallingIntoPreprocessor) {
std::string Main = R"cpp(
extern "C" {
#include "b.h"
}
)cpp";
std::string Header = R"cpp(
#define EXTERN extern
EXTERN int waldo();
)cpp";
auto TU = TestTU::withCode(Main);
TU.AdditionalFiles["b.h"] = Header;
TU.ClangTidyProvider = addTidyChecks("modernize-use-trailing-return-type");
// Check that no assertion failures occur during the build
TU.build();
}

TEST(DiagnosticsTest, Preprocessor) {
// This looks like a preamble, but there's an #else in the middle!
// Check that:
Expand Down
14 changes: 8 additions & 6 deletions lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,14 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
thread_context.r14 = read_register_u64(reg_ctx, "r14");
thread_context.r15 = read_register_u64(reg_ctx, "r15");
thread_context.rip = read_register_u64(reg_ctx, "rip");
thread_context.eflags = read_register_u32(reg_ctx, "rflags");
thread_context.cs = read_register_u16(reg_ctx, "cs");
thread_context.fs = read_register_u16(reg_ctx, "fs");
thread_context.gs = read_register_u16(reg_ctx, "gs");
thread_context.ss = read_register_u16(reg_ctx, "ss");
thread_context.ds = read_register_u16(reg_ctx, "ds");
// To make our code agnostic to whatever type the register value identifies
// itself as, we read as a u64 and truncate to u32/u16 ourselves.
thread_context.eflags = read_register_u64(reg_ctx, "rflags");
thread_context.cs = read_register_u64(reg_ctx, "cs");
thread_context.fs = read_register_u64(reg_ctx, "fs");
thread_context.gs = read_register_u64(reg_ctx, "gs");
thread_context.ss = read_register_u64(reg_ctx, "ss");
thread_context.ds = read_register_u64(reg_ctx, "ds");
return thread_context;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def verify_core_file(
expected_modules,
expected_threads,
stacks_to_sps_map,
stacks_to_registers_map,
):
# To verify, we'll launch with the mini dump
target = self.dbg.CreateTarget(None)
Expand Down Expand Up @@ -62,6 +63,17 @@ def verify_core_file(
# Try to read just past the red zone and fail
process.ReadMemory(sp - red_zone - 1, 1, error)
self.assertTrue(error.Fail(), "No failure when reading past the red zone")
# Verify the registers are the same
self.assertIn(thread_id, stacks_to_registers_map)
register_val_list = stacks_to_registers_map[thread_id]
frame_register_list = frame.GetRegisters()
for x in register_val_list:
self.assertEqual(
x.GetValueAsUnsigned(),
frame_register_list.GetFirstValueByName(
x.GetName()
).GetValueAsUnsigned(),
)

self.dbg.DeleteTarget(target)

Expand Down Expand Up @@ -93,12 +105,16 @@ def test_save_linux_mini_dump(self):
expected_number_of_threads = process.GetNumThreads()
expected_threads = []
stacks_to_sp_map = {}
stakcs_to_registers_map = {}

for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
thread_id = thread.GetThreadID()
expected_threads.append(thread_id)
stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP()
stakcs_to_registers_map[thread_id] = thread.GetFrameAtIndex(
0
).GetRegisters()

# save core and, kill process and verify corefile existence
base_command = "process save-core --plugin-name=minidump "
Expand All @@ -110,6 +126,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

self.runCmd(base_command + " --style=modified-memory '%s'" % (core_dirty))
Expand All @@ -120,6 +137,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

self.runCmd(base_command + " --style=full '%s'" % (core_full))
Expand All @@ -130,6 +148,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

options = lldb.SBSaveCoreOptions()
Expand All @@ -147,6 +166,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

options = lldb.SBSaveCoreOptions()
Expand All @@ -163,6 +183,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

# Minidump can now save full core files, but they will be huge and
Expand All @@ -181,6 +202,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

self.assertSuccess(process.Kill())
Expand Down Expand Up @@ -276,13 +298,16 @@ def test_save_linux_mini_dump_default_options(self):
expected_threads = []
stacks_to_sp_map = {}
expected_pid = process.GetProcessInfo().GetProcessID()
stacks_to_registers_map = {}

for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
thread_id = thread.GetThreadID()
expected_threads.append(thread_id)
stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP()

stacks_to_registers_map[thread_id] = thread.GetFrameAtIndex(
0
).GetRegisters()

# This is almost identical to the single thread test case because
# minidump defaults to stacks only, so we want to see if the
Expand All @@ -294,7 +319,14 @@ def test_save_linux_mini_dump_default_options(self):
error = process.SaveCore(options)
self.assertTrue(error.Success())

self.verify_core_file(default_value_file, expected_pid, expected_modules, expected_threads, stacks_to_sp_map)
self.verify_core_file(
default_value_file,
expected_pid,
expected_modules,
expected_threads,
stacks_to_sp_map,
stacks_to_registers_map,
)

finally:
self.assertTrue(self.dbg.DeleteTarget(target))
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Config/llvm-config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/* Indicate that this is LLVM compiled from the amd-gfx branch. */
#define LLVM_HAVE_BRANCH_AMD_GFX
#define LLVM_MAIN_REVISION 509964
#define LLVM_MAIN_REVISION 509968

/* Define if LLVM_ENABLE_DUMP is enabled */
#cmakedefine LLVM_ENABLE_DUMP
Expand Down
5 changes: 5 additions & 0 deletions llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct SimplifyCFGOptions {
bool ConvertSwitchToLookupTable = false;
bool NeedCanonicalLoop = true;
bool HoistCommonInsts = false;
bool HoistLoadsStoresWithCondFaulting = false;
bool SinkCommonInsts = false;
bool SimplifyCondBranch = true;
bool SpeculateBlocks = true;
Expand Down Expand Up @@ -59,6 +60,10 @@ struct SimplifyCFGOptions {
HoistCommonInsts = B;
return *this;
}
SimplifyCFGOptions &hoistLoadsStoresWithCondFaulting(bool B) {
HoistLoadsStoresWithCondFaulting = B;
return *this;
}
SimplifyCFGOptions &sinkCommonInsts(bool B) {
SinkCommonInsts = B;
return *this;
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Passes/PassBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,6 +848,8 @@ Expected<SimplifyCFGOptions> parseSimplifyCFGOptions(StringRef Params) {
Result.needCanonicalLoops(Enable);
} else if (ParamName == "hoist-common-insts") {
Result.hoistCommonInsts(Enable);
} else if (ParamName == "hoist-loads-stores-with-cond-faulting") {
Result.hoistLoadsStoresWithCondFaulting(Enable);
} else if (ParamName == "sink-common-insts") {
Result.sinkCommonInsts(Enable);
} else if (ParamName == "speculate-unpredictables") {
Expand Down
8 changes: 5 additions & 3 deletions llvm/lib/Passes/PassBuilderPipelines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1534,9 +1534,11 @@ PassBuilder::buildModuleOptimizationPipeline(OptimizationLevel Level,

// LoopSink (and other loop passes since the last simplifyCFG) might have
// resulted in single-entry-single-exit or empty blocks. Clean up the CFG.
OptimizePM.addPass(SimplifyCFGPass(SimplifyCFGOptions()
.convertSwitchRangeToICmp(true)
.speculateUnpredictables(true)));
OptimizePM.addPass(
SimplifyCFGPass(SimplifyCFGOptions()
.convertSwitchRangeToICmp(true)
.speculateUnpredictables(true)
.hoistLoadsStoresWithCondFaulting(true)));

// Add the core optimizing pipeline.
MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM),
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ static cl::opt<bool> UserHoistCommonInsts(
"hoist-common-insts", cl::Hidden, cl::init(false),
cl::desc("hoist common instructions (default = false)"));

static cl::opt<bool> UserHoistLoadsStoresWithCondFaulting(
"hoist-loads-stores-with-cond-faulting", cl::Hidden, cl::init(false),
cl::desc("Hoist loads/stores if the target supports conditional faulting "
"(default = false)"));

static cl::opt<bool> UserSinkCommonInsts(
"sink-common-insts", cl::Hidden, cl::init(false),
cl::desc("Sink common instructions (default = false)"));
Expand Down Expand Up @@ -326,6 +331,9 @@ static void applyCommandLineOverridesToOptions(SimplifyCFGOptions &Options) {
Options.NeedCanonicalLoop = UserKeepLoops;
if (UserHoistCommonInsts.getNumOccurrences())
Options.HoistCommonInsts = UserHoistCommonInsts;
if (UserHoistLoadsStoresWithCondFaulting.getNumOccurrences())
Options.HoistLoadsStoresWithCondFaulting =
UserHoistLoadsStoresWithCondFaulting;
if (UserSinkCommonInsts.getNumOccurrences())
Options.SinkCommonInsts = UserSinkCommonInsts;
if (UserSpeculateUnpredictables.getNumOccurrences())
Expand Down Expand Up @@ -354,6 +362,8 @@ void SimplifyCFGPass::printPipeline(
<< "switch-to-lookup;";
OS << (Options.NeedCanonicalLoop ? "" : "no-") << "keep-loops;";
OS << (Options.HoistCommonInsts ? "" : "no-") << "hoist-common-insts;";
OS << (Options.HoistLoadsStoresWithCondFaulting ? "" : "no-")
<< "hoist-loads-stores-with-cond-faulting;";
OS << (Options.SinkCommonInsts ? "" : "no-") << "sink-common-insts;";
OS << (Options.SpeculateBlocks ? "" : "no-") << "speculate-blocks;";
OS << (Options.SimplifyCondBranch ? "" : "no-") << "simplify-cond-branch;";
Expand Down
Loading

0 comments on commit c7b3b9d

Please sign in to comment.