Created attachment 136096 [details] modified files in coregrind Last one! Amongst this lot are changes that also cover https://bugs.kde.org/show_bug.cgi?id=407589 which is useful on FreeBSD (not really on Linux) Coregrind modified files. Lots of additional || defined(VGO_freebsd) aspacemgr coredump set osrel so as to not get some ancient signal behaviour hack for 2 RW LOAD segments - WIP fix a few warnings in traces machine code write stderr routines add some handling for capability mode extra signals some specific libc suntion implementations check that security.bsd.unprivileged_proc_debug sysctl is not set - it will cause failure _start function extra redirs some changes to traces some very funky code to handle different calling conventions for the TWO types of syscall return code differences detection of i386 running on amd64
Created attachment 141845 [details] coregrind modified files
Created attachment 142192 [details] coregrind modified files
We really should rename coregrind/m_aspacemgr/aspacemgr-linux.c one day... This code formatting is really ugly/unreadable: @@ -2165,13 +2248,21 @@ VG_(am_notify_client_mmap)( Addr a, SizeT len, UInt prot, UInt flags, needDiscard = any_Ts_in_range( a, len ); init_nsegment( &seg ); - seg.kind = (flags & VKI_MAP_ANONYMOUS) ? SkAnonC : SkFileC; + seg.kind = (flags & (VKI_MAP_ANONYMOUS +#if defined(VGO_freebsd) + | VKI_MAP_STACK +#endif +)) ? SkAnonC : SkFileC; seg.start = a; seg.end = a + len - 1; seg.hasR = toBool(prot & VKI_PROT_READ); seg.hasW = toBool(prot & VKI_PROT_WRITE); seg.hasX = toBool(prot & VKI_PROT_EXEC); - if (!(flags & VKI_MAP_ANONYMOUS)) { + if (!(flags & (VKI_MAP_ANONYMOUS +#if defined(VGO_freebsd) + | VKI_MAP_STACK +#endif +))) { // Nb: We ignore offset requests in anonymous mmaps (see bug #126722) seg.offset = offset; if (ML_(am_get_fd_d_i_m)(fd, &dev, &ino, &mode)) { Can't you define something like MAP_STACK_ ANONYMOUS to either VKI_MAP_ANONYMOUS or VKI_MAP_ANONYMOUS | VKI_MAP_STACK and use that? In coregrind/m_debuginfo/readelf.c there are various changes like: @@ -1939,13 +2014,13 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) for (i = 0; i < VG_(sizeXA)(di->fsm.maps); i++) { const DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i); if (map->rx) - TRACE_SYMTAB("rx_map: avma %#lx size %lu foff %ld\n", + TRACE_SYMTAB("rx_map: avma %#lx size %lu foff %lld\n", map->avma, map->size, map->foff); } for (i = 0; i < VG_(sizeXA)(di->fsm.maps); i++) { const DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i); if (map->rw) - TRACE_SYMTAB("rw_map: avma %#lx size %lu foff %ld\n", + TRACE_SYMTAB("rw_map: avma %#lx size %lu foff %lld\n", map->avma, map->size, map->foff); } @@ -2166,7 +2246,7 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) for (i = 0; i < VG_(sizeXA)(di->fsm.maps); i++) { const DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i); if (map->rx) - TRACE_SYMTAB("rx: at %#lx are mapped foffsets %ld .. %lu\n", + TRACE_SYMTAB("rx: at %#lx are mapped foffsets %lld .. %llu\n", map->avma, map->foff, map->foff + map->size - 1 ); } TRACE_SYMTAB("rx: contains these svma regions:\n"); @@ -2179,7 +2259,7 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) for (i = 0; i < VG_(sizeXA)(di->fsm.maps); i++) { const DebugInfoMapping* map = VG_(indexXA)(di->fsm.maps, i); if (map->rw) - TRACE_SYMTAB("rw: at %#lx are mapped foffsets %ld .. %lu\n", + TRACE_SYMTAB("rw: at %#lx are mapped foffsets %lld .. %llu\n", map->avma, map->foff, map->foff + map->size - 1 ); } TRACE_SYMTAB("rw: contains these svma regions:\n"); @@ -2222,7 +2302,7 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) } } - TRACE_SYMTAB(" [sec %2ld] %s %s al%4u foff %6ld .. %6lu " + TRACE_SYMTAB(" [sec %2ld] %s %s al%4u foff %6lld .. %6llu " " svma %p name \"%s\"\n", i, inrx ? "rx" : " ", inrw ? "rw" : " ", alyn, foff, (size == 0) ? foff : foff+size-1, (void *) svma, name); Which causes warnings like: m_debuginfo/readelf.c:2017:23: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 4 has type ‘OffT’ {aka ‘long int’} [-Wformat=] 2017 | TRACE_SYMTAB("rx_map: avma %#lx size %lu foff %lld\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2018 | map->avma, map->size, map->foff); | ~~~~~~~~~ | | | OffT {aka long int} m_debuginfo/priv_storage.h:1199:44: note: in definition of macro ‘TRACE_SYMTAB’ 1199 | if (TRACE_SYMTAB_ENABLED) { VG_(printf)(format, ## args); } | ^~~~~~ m_debuginfo/readelf.c:2017:63: note: format string is defined here 2017 | TRACE_SYMTAB("rx_map: avma %#lx size %lu foff %lld\n", | ~~~^ | | | long long int | %ld In file included from m_debuginfo/readelf.c:53: m_debuginfo/readelf.c:2023:23: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 4 has type ‘OffT’ {aka ‘long int’} [-Wformat=] 2023 | TRACE_SYMTAB("rw_map: avma %#lx size %lu foff %lld\n", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 2024 | map->avma, map->size, map->foff); | ~~~~~~~~~ | | | OffT {aka long int} m_debuginfo/priv_storage.h:1199:44: note: in definition of macro ‘TRACE_SYMTAB’ 1199 | if (TRACE_SYMTAB_ENABLED) { VG_(printf)(format, ## args); } | ^~~~~~ m_debuginfo/readelf.c:2023:63: note: format string is defined here 2023 | TRACE_SYMTAB("rw_map: avma %#lx size %lu foff %lld\n", | ~~~^ | | | long long int | %ld In coregrind/m_debuginfo/storage.c we have: +#if defined(VGO_freebsd) + if (sym->size == 0) + sym->size = 1; +#endif + /* Ignore zero-sized syms. */ if (sym->size == 0) return; Why? Do we not want to ignore zero-sized syms? Are those really size 1? 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? The logic and the defines feel swapped in VG_(is32on64)(void). But I might misunderstand why the check for that file is done. /* Number of file descriptors that Valgrind tries to reserve for its own use - just a small constant. */ +#if defined(VGO_freebsd) +#define N_RESERVED_FDS (20) +#else #define N_RESERVED_FDS (12) +#endif Why does FreeBSD need 8 more? I don't understand what SIGSYS is/does. This patch adds/clears it unconditionally, not just for freebsd. Is that correct? 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); +} aha, this is where the realloc support is added. Dont forget to close bug #407589. Was this deliberate? diff --git a/coregrind/m_scheduler/sema.c b/coregrind/m_scheduler/sema.c index 61e10dcf0..5954cdf8f 100644 --- a/coregrind/m_scheduler/sema.c +++ b/coregrind/m_scheduler/sema.c @@ -108,8 +108,8 @@ void ML_(sema_down)( vg_sema_t *sema, Bool as_LL ) INNER_REQUEST(ANNOTATE_RWLOCK_ACQUIRED(sema, /*is_w*/1)); if (ret != 1) - VG_(debugLog)(0, "scheduler", - "VG_(sema_down): read returned %d\n", ret); + VG_(debugLog)(1, "scheduler", + "ML_(sema_down): read returned %d\n", ret); if (ret == -VKI_EINTR) goto again; This change: @@ -2459,9 +2528,9 @@ void async_signalhandler ( Int sigNo, info->si_code = sanitize_si_code(info->si_code); if (VG_(clo_trace_signals)) - VG_(dmsg)("async signal handler: signal=%d, tid=%u, si_code=%d, " + VG_(dmsg)("async signal handler: signal=%d, vgtid=%u, tid=%u, si_code=%d, " "exitreason %s\n", - sigNo, tid, info->si_code, + sigNo, VG_(gettid)(), tid, info->si_code, VG_(name_of_VgSchedReturnCode)(tst->exitreason)); /* See similar logic in VG_(poll_signals). */ Causes this warning: m_signals.c: In function ‘async_signalhandler’: m_signals.c:2531:58: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 3 has type ‘Int’ {aka ‘int’} [-Wformat=] 2531 | VG_(dmsg)("async signal handler: signal=%d, vgtid=%u, tid=%u, si_code=%d, " | ~^ | | | unsigned int | %u
(In reply to Mark Wielaard from comment #3) > We really should rename coregrind/m_aspacemgr/aspacemgr-linux.c one day... > > This code formatting is really ugly/unreadable: { VKI_MAP_STACK } I'll see what I can do. Just defining this to 0 in FreeBSD should also work, with a comment to explain that it doesn't really exist. > - 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. > > In coregrind/m_debuginfo/storage.c we have: > > +#if defined(VGO_freebsd) > + if (sym->size == 0) > + sym->size = 1; > +#endif > > Why? Do we not want to ignore zero-sized syms? Are those really size 1? I don't know why this was done, it predates my work and I can#t see the change in any git or mercurial repo that I have access to. I'll do some more testing to see if it is still needed. > > 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. > 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. > > /* Number of file descriptors that Valgrind tries to reserve for > its own use - just a small constant. */ > +#if defined(VGO_freebsd) > +#define N_RESERVED_FDS (20) > +#else > #define N_RESERVED_FDS (12) > +#endif > > Why does FreeBSD need 8 more? Again that predates my work and I'll recheck. > I don't understand what SIGSYS is/does. This patch adds/clears it > unconditionally, not just for freebsd. Is that correct? SIGSYS gets sent for a non-existent syscall. I'll write a small test and see if it's needed on Linux and FreeBSD. > 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. > aha, this is where the realloc support is added. Dont forget to close bug > #407589. > > Was this deliberate? > > diff --git a/coregrind/m_scheduler/sema.c b/coregrind/m_scheduler/sema.c > index 61e10dcf0..5954cdf8f 100644 > --- a/coregrind/m_scheduler/sema.c > +++ b/coregrind/m_scheduler/sema.c > @@ -108,8 +108,8 @@ void ML_(sema_down)( vg_sema_t *sema, Bool as_LL ) > INNER_REQUEST(ANNOTATE_RWLOCK_ACQUIRED(sema, /*is_w*/1)); > > if (ret != 1) > - VG_(debugLog)(0, "scheduler", > - "VG_(sema_down): read returned %d\n", ret); > + VG_(debugLog)(1, "scheduler", > + "ML_(sema_down): read returned %d\n", ret); Yes, that was deliberate, it was causing too much noise with debug 0 on FreeBSD. > This change: > > @@ -2459,9 +2528,9 @@ void async_signalhandler ( Int sigNo, > info->si_code = sanitize_si_code(info->si_code); > > if (VG_(clo_trace_signals)) > - VG_(dmsg)("async signal handler: signal=%d, tid=%u, si_code=%d, " > + VG_(dmsg)("async signal handler: signal=%d, vgtid=%u, tid=%u, > si_code=%d, " > "exitreason %s\n", > - sigNo, tid, info->si_code, > + sigNo, VG_(gettid)(), tid, info->si_code, > VG_(name_of_VgSchedReturnCode)(tst->exitreason)); > > /* See similar logic in VG_(poll_signals). */ > > Causes this warning: I'll fix that.
(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. > > 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. > > 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. > > 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?
(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. As a general comment, the way I've handled these problems in the past is simply to cast the relevant values to the widest possible type -- a 64-bit signed or unsigned int (Long or ULong) and then used %lld, %llx, etc. That makes the problem go away without need of macros or other complexity, although you do have to be a bit careful about whether the cast requires signed or unsigned widening.
(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
> In coregrind/m_debuginfo/storage.c we have: > > +#if defined(VGO_freebsd) > + if (sym->size == 0) > + sym->size = 1; > +#endif The source of this has been lost in the mists of time. The oldest hg repo that I have has this as the 2nd commit to storage.c. Commit 0 is an import from p4 and commit 1 is summary: - Add FreeBSD patches. 11 years ago. I have no trace of the patch (nothing in the port git log which goes back a full 17 years). I tried removing it and it seems to have no effect on the regression tests. I think it's likely that this is no longer needed since this code must date back to when GCC was used for the toolchain. > /* Number of file descriptors that Valgrind tries to reserve for > its own use - just a small constant. */ > +#if defined(VGO_freebsd) > +#define N_RESERVED_FDS (20) > +#else > #define N_RESERVED_FDS (12) > +#endif > > Why does FreeBSD need 8 more? Exactly the same story as the previous change. > I don't understand what SIGSYS is/does. This patch adds/clears it > unconditionally, not just for freebsd. Is that correct? Time to throw in the towel tonight, I'll look at that one tomorrow.
> Commit 0 is an import from p4 I believe we still have the p4 repo backed somewhere but the server is down, without any easy way to restore it. It's probably not worth it to try to find out the origin of these little snippets. > I think it's likely that this is no longer needed since this code must date > back to when GCC was used for the toolchain. That's probably the case - we never moved beyond a 2006 or so GNU toolchain (incl binutils).
> > I don't understand what SIGSYS is/does. This patch adds/clears it > > unconditionally, not just for freebsd. Is that correct? > It doesn't seem to make any difference. A little test that uses syscall 600 (not quite there yet on FreeBSD) generates, standalone Bad system call(coredump) And under Valgrind (both with and without the handler/restore) --12667-- WARNING: unhandled amd64-freebsd syscall: 600 That seems good enough to me so I'll remove those two calls.
Code committed with commit 68bb7c063f71631a4f207adca2235eb0f8d00d33