Page MenuHomePhabricator

Fix function name lookup in IRExecutionEngine.cpp.
ClosedPublic

Authored by sas on May 16 2016, 4:27 PM.

Details

Summary

Without this commit, when log enable lldb expr is enabled, the
disassembly of JIT'ed code is never displayed.

Diff Detail

Repository
rL LLVM

Event Timeline

sas updated this revision to Diff 57418.May 16 2016, 4:27 PM
sas retitled this revision from to Fix function name lookup in IRExecutionEngine.cpp..
sas updated this object.
sas added a reviewer: spyffe.
sas added a subscriber: lldb-commits.
clayborg requested changes to this revision.May 19 2016, 12:43 PM
clayborg edited edge metadata.

See inlined comments.

source/Expression/IRExecutionUnit.cpp
128 ↗(On Diff #57418)

If "function.m_name" is a lldb_private::ConstString and so it "m_name", then please do this comparison as:

if (function.m_name == m_name)

Check with Sean to see if changing != to == is the correct thing.

This revision now requires changes to proceed.May 19 2016, 12:43 PM
sas added a comment.May 19 2016, 1:37 PM

Ah yes, good idea not to use AsCString() here.

I think changing the != to == is the right thing to do here because the bug was introduced in r263995:

@@ -122,7 +125,7 @@ IRExecutionUnit::DisassembleFunction (Stream &stream,

     for (JittedFunction &function : m_jitted_functions)
     {
-        if (strstr(function.m_name.c_str(), m_name.AsCString()))
+        if (function.m_name.AsCString() != m_name.AsCString())
         {
             func_local_addr = function.m_local_addr;
             func_remote_addr = function.m_remote_addr;

The previous code was using strstr to check if the function names matched, but has been changed to !=; probably a typo assuming we were doing strcmp instead of strstr.

I'll wait for @spyffe to confirm.

sas updated this revision to Diff 57854.May 19 2016, 1:37 PM
sas edited edge metadata.

No need to use .AsCString() on these.

clayborg accepted this revision.May 19 2016, 2:32 PM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.May 19 2016, 2:32 PM
This revision was automatically updated to reflect the committed changes.