memcheck/tests/varinfo5 is failing [FreeBSD 12.2 and later]

[paulfloyd]

Since adding a hack for an extra elf segment, this test is failing. It's probably related to there being a dynamic library. So quite important.

Before the error message contained

Location 0x........ is 0 bytes inside global var "global_u1" declared at varinfo5so.c:38

and now

Address 0x........ is in a rw- mapped file /usr/home/paulf/scratch/valgrind/memcheck/tests/varinfo5so.so segment

[paulfloyd]

Asked about this on the freebsd toolchain list and got this reply.

On 19 Feb 2021, at 15:18, Paul Floyd pjfloyd@wanadoo.fr wrote:

A while back when I upgraded to FreeBSD 12.2 (and thus to clang 10) I got quite a new category of errors with Valgrind.

The problem is that the clang 10 toolchain produces two RW LOAD segments, for instance see below. Valgrind assumes that there is only one, and ignores the second one which results in false positives when reading PLTs. I've added a hack to make it seem like there is just one such segment, but it isn't 100% reliable - there's at least one issue when loading shared libraries.

This changed in lld 9.0.0, with upstream r356226 (aka https://github.com/llvm/llvm-project/commit/e8710ef1fbe8109eaa36143654f325dd345f8a0133 )

commit e8710ef1fbe8109eaa36143654f325dd345f8a01 Author: Fangrui Song maskray@google.com Date: Fri Mar 15 01:29:57 2019 +0000

[ELF] Split RW PT_LOAD on the PT_GNU_RELRO boundary

Summary:
Based on Peter Collingbourne's suggestion in D56828.

Before D56828: PT_LOAD(.data PT_GNU_RELRO(.data.rel.ro .bss.rel.ro) .bss)
Old:           PT_LOAD(PT_GNU_RELRO(.data.rel.ro .bss.rel.ro) .data .bss)
New:           PT_LOAD(PT_GNU_RELRO(.data.rel.ro .bss.rel.ro)) PT_LOAD(.data. .bss)

The new layout reflects the runtime memory mappings.
By having two PT_LOAD segments, we can utilize the NOBITS part of the
first PT_LOAD and save bytes for .bss.rel.ro.

.bss.rel.ro is currently small and only used by copy relocations of
symbols in read-only segments, but it can be used for other purposes in
the future, e.g. if a relro section's statically relocated data is all
zeros, we can move it to .bss.rel.ro.

Reviewers: espindola, ruiu, pcc

Reviewed By: ruiu

Subscribers: nemanjai, jvesely, nhaehnle, javed.absar, kbarton, emaste, arichardson, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D58892

llvm-svn: 356226

In other words, this is shuffling the segments around a bit to achieve a more optimal layout for relro.

I will carry on looking for a proper solution. In the meantime, are there any flags to revert to the previous behaviour and only generate a single RW LOAD segment?

I think valgrind should be fixed to able to cope with additional segments, but I haven't seen valgrind working on FreeBSD for years now, so I am not going to hold my breath.

That said, you can attempt to link your executables with -z norelro (or -Wl,-z,norelro via the compiler driver). If there is no PT_GNU_RELRO header, lld will not split the segments.

And of course, you can link with lld 8.0 if all else fails.

-Dimitry If I build varinfo5 and .so with the above options then the testcase gived good results (the testcase would need to have the expected updated).

[paulfloyd]

The hacky fix that I want to replace is

#if (FREEBSD_VERS >= FREEBSD_12_2)
                     if ((long long int)item.bias < 0LL) {
                        item.bias = 0;
                     }
#endif

readelf.c line 2117

in Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di )

And the real cause of the problem is

debuginfo.c

di = find_or_create_DebugInfo_for( filename );

line 1269

if (di->havedinfo) { if (debug) VG(dmsg)("di_notify_mmap-4x: " "ignoring mapping because we already read debuginfo " "for DebugInfo* %p\n", di); return 0; }

which gives things like (with -d -d -d )

--1982-- di_notify_mmap-2: /usr/home/paulf/scratch/valgrind/memcheck/tests/varinfo5 --1982-- di_notify_mmap-3: is_rx_map 0, is_rw_map 1, is_ro_map 0 --1982-- di_notify_mmap-4x: ignoring mapping because we already read debuginfo for DebugInfo* 0x40287C220

[paulfloyd]

The more I look at this, the less I seem to understand.

maybe_merge_nsegments should be merging these segments, but it looks like the offsets are phoney

[paulfloyd]

Merge doesn't seem to help much

Next look at pp_addrinfo_WRK to see why nosplit hits Addr_DataSym but split only Addr_SectKind

This is the code snippet:

   if (VG_(get_datasym_and_offset)(
             ep, a, &name,
             &ai->Addr.DataSym.offset )) {
      ai->Addr.DataSym.name = VG_(strdup)("mc.da.dsname", name);
      ai->tag = Addr_DataSym;
      return;
   }

[paulfloyd]

I'm now tracking progress in the kde bugzilla

https://bugs.kde.org/show_bug.cgi?id=452802

Hopefully I'll get something that works tonight.

[paulfloyd]

commit 7844752299b5472b21fc4df765d4cffdf92c6c3d (HEAD -> master, origin/master, origin/HEAD) Author: Paul Floyd pjfloyd@wanadoo.fr Date: Thu Jun 9 22:03:04 2022 +0200

Bug 452802 Handle lld 9+ split RW PT_LOAD segments correctly

Many changes mostly related to modifying VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
so that instead of triggering debuginfo reading after seeing one RX PT_LOAD and 1 RW PT_LOAD it