Re: [lldb-dev] Where "thread until " should set breakpoints?

[Venkata Ramanaiah Nalamothu via lldb-dev]

I have created https://reviews.llvm.org/D50304.

BTW, I almost forgot the fact that the error message changes were already
covered in my other patch https://reviews.llvm.org/D48865.

On Fri, Aug 3, 2018 at 11:41 PM, Jim Ingham  wrote:

> Thanks!  I look forward to the patch.
>
> Jim
>
>
> > On Aug 2, 2018, at 8:56 PM, Venkata Ramanaiah Nalamothu <
> ramana.venka...@gmail.com> wrote:
> >
> > Thanks Jim for the elaborate reply.
> >
> > I knew what is happening in below piece of code and also has a patch
> ready but now needs a bit of fine tuning based on your below comments. I
> just wanted to hear from you folks that my understanding is correct and I
> am not missing something.
> >
> > Also, the code around it needs modification of error messages to refer
> to 'thread->GetID()' instead of 'm_options.m_thread_idx'. I will create a
> review on phabricator with all these changes.
> >
> > - Ramana
> >
> > On Thu, Aug 2, 2018 at 10:56 PM, Jim Ingham  wrote:
> > That's a bug.  The code in CommandObjectThreadUntil needs to be a little
> more careful about sliding the line number to the "nearest source line that
> has code."  It currently does:
> >
> > for (uint32_t line_number : line_numbers) {
> >   uint32_t start_idx_ptr = index_ptr;
> >   while (start_idx_ptr <= end_ptr) {
> > LineEntry line_entry;
> > const bool exact = false;
> > start_idx_ptr = sc.comp_unit->FindLineEntry(
> > start_idx_ptr, line_number, sc.comp_unit, exact,
> &line_entry);
> > if (start_idx_ptr == UINT32_MAX)
> >   break;
> >
> > addr_t address =
> > line_entry.range.GetBaseAddress().
> GetLoadAddress(target);
> > if (address != LLDB_INVALID_ADDRESS) {
> >   if (fun_addr_range.ContainsLoadAddress(address, target))
> > address_list.push_back(address);
> >   else
> > all_in_function = false;
> > }
> > start_idx_ptr++;
> >   }
> > }
> >
> > The problem is that in the while loop we set:
> >
> > const bool exact = false;
> >
> > Having exact == false through the whole loop means that all the other
> line table entries after the last exact match will match because from the
> index ptr on there are no more entries, so we slide it to the next one.
> >
> > In general it's not always easy to tell which lines will actually
> contribute code so lldb is lax about line number matching, and slides the
> breakpoint to the nearest subsequent line that has code when there's no
> exact match.  That seems a good principle to follow here as well.  So I
> don't want to just set exact to "true".
> >
> > I think the better behavior is to try FindLineEntry the first time with
> exact = false.  If that picks up a different line from the one given, reset
> the line we are looking for to the found line.  In either case, then go on
> to search for all the other instances of that line with exact = false.  It
> might actually be handy to have another function on CompUnit like
> FindAllEntriesForLine that bundles up this behavior as it seems a generally
> useful one.
> >
> > If you want to give this a try, please do.  Otherwise, file a bug and
> I'll get to it when I have a chance.
> >
> > Jim
> >
> >
> >
> > > On Aug 1, 2018, at 10:22 PM, Ramana  wrote:
> > >
> > >
> > > On Thu, Aug 2, 2018 at 3:32 AM, Jim Ingham  wrote:
> > >
> > >
> > > > On Jul 24, 2018, at 9:05 PM, Ramana via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> > > >
> > > > On the subject line, the ToT lldb (see code around
> CommandObjectThread.cpp:1230) sets the breakpoint on the first exact
> matching line of 'line-number' or the closest line number > 'line-number'
> i.e. the best match.
> > > >
> > > > And along with that, starting from the above exact/best matching
> line number index in the line table, the breakpoints are also being set on
> every other line number available in the line table in the current function
> scope. This latter part, I believe, is incorrect.
> > >
> > > Why do you think this is incorrect?
> > >
> > > The requirements for "thread until " are:
> > >
> > > a) If any code contributed by  is executed before leaving
> the function, stop
> > > b) If you end up leaving the function w/o triggering (a), then stop
> > >
> > > Understood and no concerns on this.
> > >
> > > Correct or incorrect should be determined by how well the
> implementation fits those requirements.
> > >
> > > There isn't currently a reliable indication from the debug information
> or line tables that "line N will always be entered starting with the block
> at 0x123".  So you can't tell without doing control flow analysis, which if
> any of the separate entries in the line table for the same line will get
> hit in the course of executing the function.  So the safest thing to do is
> to set breakpoints on them all.
> > >
> > > From the above, I understand that we have t

Re: [lldb-dev] [Release-testers] [7.0.0 Release] rc1 has been tagged

[Dimitry Andric via lldb-dev]

On 3 Aug 2018, at 13:37, Hans Wennborg via Release-testers 
 wrote:
> 
> 7.0.0-rc1 was just tagged (from the branch at r338847).
> 
> It's early in the release process, but I'd like to find out what the
> status is of the branch on our various platforms.
> 
> Please run the test script, share the results, and upload binaries.

Hmm, I'm running into a rather nasty snag now on i386-freebsd11, due to our 
lack of atomic 64 bit primitives; Phase2's configure dies with:

-- Performing Test HAVE_CXX_ATOMICS_WITHOUT_LIB
-- Performing Test HAVE_CXX_ATOMICS_WITHOUT_LIB - Success
-- Performing Test HAVE_CXX_ATOMICS64_WITHOUT_LIB
-- Performing Test HAVE_CXX_ATOMICS64_WITHOUT_LIB - Failed
-- Looking for __atomic_load_8 in atomic
-- Looking for __atomic_load_8 in atomic - not found
CMake Error at cmake/modules/CheckAtomic.cmake:75 (message):
  Host compiler appears to require libatomic, but cannot find it.

Interestingly, Phase1 does *not* suffer from this, but there the "host 
compiler" is clang 6.0.0.

Phase1 CMake output:

Performing C++ SOURCE FILE Test HAVE_CXX_ATOMICS64_WITHOUT_LIB succeeded 
with the following output:
Change Dir: 
/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp

Run Build Command:"/usr/local/bin/gmake" "cmTC_845df/fast"
/usr/local/bin/gmake -f CMakeFiles/cmTC_845df.dir/build.make 
CMakeFiles/cmTC_845df.dir/build
gmake[1]: Entering directory 
'/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_845df.dir/src.cxx.o
/usr/bin/c++-DHAVE_CXX_ATOMICS64_WITHOUT_LIB -std=c++11  
-Werror=unguarded-availability-new   -o CMakeFiles/cmTC_845df.dir/src.cxx.o -c 
/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp/src.cxx
Linking CXX executable cmTC_845df
/usr/local/bin/cmake -E cmake_link_script 
CMakeFiles/cmTC_845df.dir/link.txt --verbose=1
/usr/bin/c++   -DHAVE_CXX_ATOMICS64_WITHOUT_LIB -std=c++11  
-Werror=unguarded-availability-newCMakeFiles/cmTC_845df.dir/src.cxx.o  -o 
cmTC_845df -lm
gmake[1]: Leaving directory 
'/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp'

Source file was:

#include 
#include 
std::atomic x (0);
int main() {
  uint64_t i = x.load(std::memory_order_relaxed);
  return 0;
}

Phase2 CMake output:

Performing C++ SOURCE FILE Test HAVE_CXX_ATOMICS64_WITHOUT_LIB failed with 
the following output:
Change Dir: 
/home/dim/llvm/7.0.0/rc1/Phase2/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp

Run Build Command:"/usr/local/bin/gmake" "cmTC_720f3/fast"
/usr/local/bin/gmake -f CMakeFiles/cmTC_720f3.dir/build.make 
CMakeFiles/cmTC_720f3.dir/build
gmake[1]: Entering directory 
'/home/dim/llvm/7.0.0/rc1/Phase2/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp'
Building CXX object CMakeFiles/cmTC_720f3.dir/src.cxx.o

/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.install/usr/local/bin/clang++
-DHAVE_CXX_ATOMICS64_WITHOUT_LIB -std=c++11  
-Werror=unguarded-availability-new   -o CMakeFiles/cmTC_720f3.dir/src.cxx.o -c 
/home/dim/llvm/7.0.0/rc1/Phase2/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp/src.cxx
Linking CXX executable cmTC_720f3
/usr/local/bin/cmake -E cmake_link_script 
CMakeFiles/cmTC_720f3.dir/link.txt --verbose=1

/home/dim/llvm/7.0.0/rc1/Phase1/Release/llvmCore-7.0.0-rc1.install/usr/local/bin/clang++
   -DHAVE_CXX_ATOMICS64_WITHOUT_LIB -std=c++11  
-Werror=unguarded-availability-newCMakeFiles/cmTC_720f3.dir/src.cxx.o  -o 
cmTC_720f3 -lm
CMakeFiles/cmTC_720f3.dir/src.cxx.o: In function `main':
src.cxx:(.text+0x33): undefined reference to `__atomic_load_8'
clang-7: error: linker command failed with exit code 1 (use -v to see 
invocation)
gmake[1]: *** [CMakeFiles/cmTC_720f3.dir/build.make:98: cmTC_720f3] Error 1
gmake[1]: Leaving directory 
'/home/dim/llvm/7.0.0/rc1/Phase2/Release/llvmCore-7.0.0-rc1.obj/CMakeFiles/CMakeTmp'
gmake: *** [Makefile:126: cmTC_720f3/fast] Error 2

Source file was:

#include 
#include 
std::atomic x (0);
int main() {
  uint64_t i = x.load(std::memory_order_relaxed);
  return 0;
}

Apparently, with clang 6.0 it didn't generate libcalls to atomic functions, but 
just put in cmpxchg8b's, I guess?  And with clang 7.0 this seems to have 
changed.

For now, I can only test on amd64 due to this, since I don't have an easy 
workaround.

-Dimitry



signature.asc
Description: Message signed with OpenPGP
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] Handling of the ELF files missing build-ids?

[Pavel Labath via lldb-dev]

Hello Leonard,

while I'm in principle supportive of this idea, I think it's not going
to be as easy as you might imagine. There are currently at least two
mechanisms which rely on this crc UUID.

1. .gnu_debuglink separate file pointer
.
This is where the choice of the crc algorithm comes from.

In short, this mechanism for debug info location works like this: The
stripped file contains a .gnu_debuglink section.  The section contains
a file path and a crc checksum. After reading this section the
debugger is expected to look for the file at the given path, and then
compute it's checksum to verify it is indeed the correct file (hasn't
been modified).

In LLDB, this is implemented somewhat differently. First we have a
mechanism for assigning UUIDs to modules. This mechanism does one of
two things (I'm assuming here none of the files have proper build-ids
in them). If the file has a .gnu_debuglink section (the stripped
file), we fetch the CRC from there, and assign that as the build-id of
the file. If the file doesn't have this section, we **assume** it is
going to be a target of gnu_debuglink pointer in another file, compute
its CRC via the specified algorithm, and assign that as the build-id.

Second, we have a mechanism for locating the unstripped file. This
just fetches the path from gnu_debuglink, and compares the UUIDs of
the two modules. This works because of the UUID algorithm in the first
part.

Now, this is not a particularly smart way of doing things and it is
the cause of the most of your problems. However, it is completely
avoidable. Instead of piggy-backing on the UUID mechanism, we should
just change the search mechanism (the second part) to compute the crc
itself, and only after it successfully finds the file referenced by
the gnu_debuglink section. This way, we will only compute the crc only
when absolutely necessary (i.e. for your use case, never).


2. Currently, for remote debugging, we assume that each module has
some sort of a unique identifier which we can check to see whether we
have downloaded that file already (see qModuleInfo packet). I am not
sure what would happen if we suddenly just stopped computing a UUID
for these files, but we at the very least lose the ability to detect
changes to the remote files. For this item, I am not sure what would
be the best course of action. Maybe we should just start relying on
modification timestamp for these files?

On Sat, 4 Aug 2018 at 02:17, Leonard Mosescu  wrote:
>
> Greg, Mark,
>
> Looking at the code, LLDB falls back to a full file crc32 to create the 
> module UUID if the ELF build-id is missing. This works, in the sense that the 
> generated UUID does indeed identify the module.
>
> But there are a few problems with this approach:
>
> 1. First, runtime performance: a full file crc32 is a terribly inefficient 
> way to generate a temporary UUID that is basically just used to match a local 
> file to itself.
> - especially when some unstripped binaries can be very large. for example a 
> local chromium build produces a 5.3Gb chrome binary
> - the crc32 implementation is decent, but single-threaded
> - to add insult to the injury, it seems a small bug defeats the intention to 
> cache the hash value so it ends up being recalculated multiple times
>
> 2. The fake UUID is not going to match any external UUID that may be floating 
> around (and yet not properly embedded into the binary)
> - an example is Breakpad, which unfortunately also attempts to make up UUIDs 
> when the build-id is missing (something we'll hopefully fix soon)
>
> Is there a fundamental reason to calculate the full file crc32? If not I 
> propose to improve this based on the following observations:
>
> A. Model the reality more accurately: an ELF w/o a build-id doesn't really 
> have an UUID. So use a zero-length UUID in LLDB.
> B. The full file name should be enough to prove the identity of a local 
> module.
> C. If we try to match an external UUID (ex. from a minidump) with a local 
> file which does not have an UUID it may help to have an option to allow it to 
> match (off by default, and only if there's no better match)

I think we might have something already which could serve this
purpose. Eugene added a couple of months ago a mechanism to force-load
symbols for a given file regardless of the UUIDs
. It requires an explicit "target
symbols add" command (which seems reasonable, as lldb has no way to
tell if it's doing things right). Would something like that work for
you?

cheers,
pl
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev