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

--- Comment #7 from Paul Floyd <pjfl...@wanadoo.fr> ---
(In reply to Mark Wielaard from comment #5)
> (In reply to Paul Floyd from comment #4)
> > > -         TRACE_SYMTAB("rx_map:  avma %#lx   size %lu  foff %ld\n",
> > > +         TRACE_SYMTAB("rx_map:  avma %#lx   size %lu  foff %lld\n",
> > 
> > ^^^ generates warnings
> > 
> > I think that both versions probably generate warnings on either 32bit or
> > 64bit systems, and that we should use a macro to choose l or ll.
> 
> I don't think so. I believe the actual pointers and offsets are either
> 32/64bit already and match the lx/lu/ld directives.

DebugInfoMapping::size is SizeT and DebugInfoMapping::foff is OffT

For SizeT %zu is what I would usually use.
OffT is more complicated. On Linux and Solaris it is Word, but on FreeBSD and
MacOS it is Long. So what I was thinking was to add a couple of macros

#define FMT_SIZET "zu"
#if defined(VGO_linux)
#define FMT_OFFT  "ld"
#elif defined(VGO_freebsd)
#define FMT_OFFT  "lld"
#endif



> 
> > > coregrind/pub_core_gdbserver.h What is the padding in padding? I see it is
> > > actually set in coregrind/m_gdbserver/remote-utils.c remote_open. But why?
> > 
> > I don't remember exactly what the problem was, but this is the implicit
> > padding that is there anyway. The memory gets initialized with brace
> > intialization leaving the padding uninitialized. This code makes sure that
> > the entire struct gets initialized, even the unused last 4 bytes.
> 
> It seems harmless, but odd.
> If you really want to initialize the whole struct (including padding, but
> why?) then memset(p, 0, sizeof(Struct)) should do it.

This was something that was quite hard to reproduce (it involved signals when
under vgdb). I'll probably leave it as it's harmless, but add a comment.

> > > The logic and the defines feel swapped in VG_(is32on64)(void). But I might
> > > misunderstand why the check for that file is done.
> > 
> > 
> > It's very hard to tell when a 32bit process is running on an amd64 kernel
> > (for instance, all sysctls behave like a true 32bit system). When compiling
> > the amd64 binary it returns false - that can only ever be 64on64. When
> > compiling for x86 that is either on an x86 system or and amd64 system
> > building both amd64 and x86. I'm using the link loader to tell them apart.
> > This isn't totally bomb proof. Someone could try to run an x86 built
> > Valgrind on amd64 or add a /libexec/ld-elf32.so.1 on an x86 system, but I'm
> > not going to try to handle user foolishness like that.
> 
> Aha. So it is about whether the 32bit x86 freebsd valgrind runs under a 64
> amd64 kernel?
> On most GNU/Linux distros that is fine and it is actually how 32bit programs
> are run under valgrind on a 64bit system.

On Linux, you can set LD_PRELOAD to point to both 32 and 64bit libs and the
link loader will just ignore the ones of the wrong word size. FreeBSD is
fussier. On an x86 kernel, LD_PRELOAD is used for 32 bit libs. On a 64 bit
kernel, LD_PRELOAD is used for 64 bit libs and LD_32_PRELOAD for 32 bit libs.
You can't use LD_PRELOAD with x86 executable. And if the LD_PRELOAD is wrong,
none of the vgpreload_*-x86-freebsd.so libs will get loaded, breaking many many
things.

> > > Why this in coregrind/m_main.c
> > > 
> > > +void *memcpy(void *dest, const void *src, size_t n);
> > > +void *memcpy(void *dest, const void *src, size_t n) {
> > > +   return VG_(memcpy)(dest, src, n);
> > > +}
> > > +void* memmove(void *dest, const void *src, SizeT n);
> > > +void* memmove(void *dest, const void *src, SizeT n) {
> > > +   return VG_(memmove)(dest,src,n);
> > > +}
> > > +void* memset(void *s, int c, SizeT n);
> > > +void* memset(void *s, int c, SizeT n) {
> > > +  return VG_(memset)(s,c,n);
> > > +}
> > 
> > 
> > That's clang's fault. clang can convert things like
> > 
> > structA = {0};
> > and
> > structA = structB;
> > 
> > into memset and memcoy calls, so FreeBSD needs them in the tool exe.
> 
> That is really odd. Aren't we preventing that somehow with some compile flag.
> It also seems pretty inefficient, and inlined memcpy should really be faster
> for a simple struct copy than a function call to memcpy.
> But if so, could you add this to coregrind/m_compiler.c hidden behind and
> appropriate #ifdef CLANG option?

We do build with something like -nostdlib (or -fno-builtin, need to check).

This code only gets generated once or twice in the whole codebase, for the very
largest structures.

Linux has the same sort of thing.

See this discussion if you are interested
https://stackoverflow.com/questions/64648300/how-to-prevent-gcc-from-inserting-memset-during-link-time-optimization

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to