From 9f486241efac1d0152d88797dedf22f9890ac9a0 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Fri, 16 Oct 2026 17:10:21 -0700 Subject: [PATCH 1/5] Allow Fjalar's two pub_core includes in check_headers_and_includes fjalar/mc_main.c includes pub_core_aspacemgr.h (for am_is_valid_for_client) and fjalar/typedata.c includes pub_core_mallocfree.h. These are deliberate, but they made the post-regtest header check fail on every run. Allow exactly those two includes; any other pub_core include in a tool file is still an error. --- doc/valgrind-merge.texinfo | 5 ++++- valgrind/tests/check_headers_and_includes | 12 ++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/doc/valgrind-merge.texinfo b/doc/valgrind-merge.texinfo index f71a2478..5be3dcd4 100644 --- a/doc/valgrind-merge.texinfo +++ b/doc/valgrind-merge.texinfo @@ -1167,7 +1167,10 @@ Add fjalar to the list of tools; add files to clean-local target @item $INV/fjalar/valgrind/none/tests/Makefile.am Add @nospellcheck{coregrind/libcoregrind-amd64-linux.a to libvex} tests @item $INV/fjalar/valgrind/tests/check_headers_and_includes -Add fjalar to @nospellcheck{tool_dirs and inst to dirs_to_ignore} +Add fjalar to @nospellcheck{tool_dirs and inst to dirs_to_ignore}; +add @nospellcheck{tool_core_include_allowed}, which allows +@file{fjalar/mc_main.c} and @file{fjalar/typedata.c} to include the +@file{pub_core_*.h} headers they need @item $INV/fjalar/valgrind/VEX/priv/guest_amd64_helpers.c Add additional general purpose registers to the guest state @item $INV/fjalar/valgrind/VEX/priv/guest_x86_helpers.c diff --git a/valgrind/tests/check_headers_and_includes b/valgrind/tests/check_headers_and_includes index 100bd66c..deec967d 100755 --- a/valgrind/tests/check_headers_and_includes +++ b/valgrind/tests/check_headers_and_includes @@ -83,6 +83,13 @@ my %tool_export_header = ( "cachegrind/cachegrind.h" => 1 ); +# Tool files that are allowed to include particular pub_core_...h headers. +# Fjalar needs a few core facilities that have no pub_tool_...h interface. +my %tool_core_include_allowed = ( + "fjalar/mc_main.c" => { "../coregrind/pub_core_aspacemgr.h" => 1 }, + "fjalar/typedata.c" => { "../coregrind/pub_core_mallocfree.h" => 1 }, + ); + my $usage=< Date: Fri, 16 Oct 2026 19:31:30 -0700 Subject: [PATCH 2/5] Declare Fjalar's two core functions locally instead of including pub_core headers Fjalar needs VG_(am_is_valid_for_valgrind) and VG_(arena_free), which have no pub_tool_*.h interface. Declaring them in mc_main.c and typedata.c satisfies check_headers_and_includes without modifying that upstream Valgrind file, so Valgrind merges need not carry an extra change. --- doc/valgrind-merge.texinfo | 5 +---- valgrind/fjalar/mc_main.c | 4 +++- valgrind/fjalar/typedata.c | 8 ++++++-- valgrind/tests/check_headers_and_includes | 12 ++---------- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/doc/valgrind-merge.texinfo b/doc/valgrind-merge.texinfo index 5be3dcd4..f71a2478 100644 --- a/doc/valgrind-merge.texinfo +++ b/doc/valgrind-merge.texinfo @@ -1167,10 +1167,7 @@ Add fjalar to the list of tools; add files to clean-local target @item $INV/fjalar/valgrind/none/tests/Makefile.am Add @nospellcheck{coregrind/libcoregrind-amd64-linux.a to libvex} tests @item $INV/fjalar/valgrind/tests/check_headers_and_includes -Add fjalar to @nospellcheck{tool_dirs and inst to dirs_to_ignore}; -add @nospellcheck{tool_core_include_allowed}, which allows -@file{fjalar/mc_main.c} and @file{fjalar/typedata.c} to include the -@file{pub_core_*.h} headers they need +Add fjalar to @nospellcheck{tool_dirs and inst to dirs_to_ignore} @item $INV/fjalar/valgrind/VEX/priv/guest_amd64_helpers.c Add additional general purpose registers to the guest state @item $INV/fjalar/valgrind/VEX/priv/guest_x86_helpers.c diff --git a/valgrind/fjalar/mc_main.c b/valgrind/fjalar/mc_main.c index 51da087c..e2e869d2 100644 --- a/valgrind/fjalar/mc_main.c +++ b/valgrind/fjalar/mc_main.c @@ -69,7 +69,9 @@ #include "kvasir/kvasir_main.h" #include "kvasir/dyncomp_main.h" #include "fjalar_main.h" -#include "../coregrind/pub_core_aspacemgr.h" // needed for am_is_valid_for_client +// Declared in coregrind/pub_core_aspacemgr.h, which tool files must not +// include (see tests/check_headers_and_includes). +extern Bool VG_(am_is_valid_for_valgrind)(Addr start, SizeT len, UInt prot); #include "mc_include.h" #include "memcheck.h" /* for client requests */ diff --git a/valgrind/fjalar/typedata.c b/valgrind/fjalar/typedata.c index b96e81ee..95cdc27f 100644 --- a/valgrind/fjalar/typedata.c +++ b/valgrind/fjalar/typedata.c @@ -39,8 +39,12 @@ // For name demangling. #include "../coregrind/m_demangle/demangle.h" -// For VG_(arena_free), to release the demanglers' results. -#include "../coregrind/pub_core_mallocfree.h" +// For releasing the demanglers' results. These are declared in +// coregrind/pub_core_mallocfree.h, which tool files must not include (see +// tests/check_headers_and_includes). VG_AR_DEMANGLE must match the value +// there. +extern void VG_(arena_free)(Int arena, void* ptr); +#define VG_AR_DEMANGLE 3 // Most of the following information is taken from "DWARF Debugging // Information Format Version 4". Published by the DWARF Debugging diff --git a/valgrind/tests/check_headers_and_includes b/valgrind/tests/check_headers_and_includes index deec967d..100bd66c 100755 --- a/valgrind/tests/check_headers_and_includes +++ b/valgrind/tests/check_headers_and_includes @@ -83,13 +83,6 @@ my %tool_export_header = ( "cachegrind/cachegrind.h" => 1 ); -# Tool files that are allowed to include particular pub_core_...h headers. -# Fjalar needs a few core facilities that have no pub_tool_...h interface. -my %tool_core_include_allowed = ( - "fjalar/mc_main.c" => { "../coregrind/pub_core_aspacemgr.h" => 1 }, - "fjalar/typedata.c" => { "../coregrind/pub_core_mallocfree.h" => 1 }, - ); - my $usage=< Date: Fri, 16 Oct 2026 19:44:40 -0700 Subject: [PATCH 3/5] Put Fjalar's core declarations in one header, checked at build time fjalar_core_decls.h holds the copies of VG_(am_is_valid_for_valgrind), VG_(arena_free), and VG_AR_DEMANGLE. A rule in fjalar/Makefile.am compiles that header together with pub_core_aspacemgr.h and pub_core_mallocfree.h, so the build fails if a Valgrind merge changes a declaration. Document this in valgrind-merge.texinfo. --- .gitignore | 3 +++ doc/valgrind-merge.texinfo | 9 +++++++ valgrind/fjalar/Makefile.am | 28 ++++++++++++++++++++ valgrind/fjalar/fjalar_core_decls.h | 40 +++++++++++++++++++++++++++++ valgrind/fjalar/mc_main.c | 4 +-- valgrind/fjalar/typedata.c | 8 ++---- 6 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 valgrind/fjalar/fjalar_core_decls.h diff --git a/.gitignore b/.gitignore index 74501df4..686d4d83 100644 --- a/.gitignore +++ b/.gitignore @@ -735,6 +735,9 @@ valgrind/fjalar/rundoxy # Ignore shell scripts for compiling fjalar valgrind/fjalar/*.sh +# Stamp file for the fjalar_core_decls.h consistency check +valgrind/fjalar/fjalar_core_decls.stamp + # Documentation valgrind-merge.aux valgrind-merge.cp diff --git a/doc/valgrind-merge.texinfo b/doc/valgrind-merge.texinfo index f71a2478..4943ddeb 100644 --- a/doc/valgrind-merge.texinfo +++ b/doc/valgrind-merge.texinfo @@ -495,6 +495,15 @@ changes in Valgrind API/functionality. For the most part Valgrind maintains a relatively stable interface to its tools. Any tool-visible changes should be noted in the change logs. +Fjalar also uses a few Valgrind core entities that are not part of the +tool interface. Tool files may not include the core headers that +declare them, so @file{fjalar/fjalar_core_decls.h} contains copies of +those declarations. The build compiles those copies together with the +core headers, so if the build fails in the rule for +@file{fjalar_core_decls.stamp}, then a core declaration has changed. +Update the copy in @file{fjalar/fjalar_core_decls.h} to match the core +header that it names. + A somewhat more problematic area are changes in the VEX IR. DynComp makes heavy use of the VEX IR, so any changes in it need to be reflected. Most of the changes to the public VEX IR can be discovered diff --git a/valgrind/fjalar/Makefile.am b/valgrind/fjalar/Makefile.am index a7ae83ad..e8e803a9 100644 --- a/valgrind/fjalar/Makefile.am +++ b/valgrind/fjalar/Makefile.am @@ -177,3 +177,31 @@ vgpreload_fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_so_LDFLAGS = \ $(LIBREPLACEMALLOC_LDFLAGS_@VGCONF_PLATFORM_SEC_CAPS@) endif + +#---------------------------------------------------------------------------- +# Consistency check for fjalar_core_decls.h +#---------------------------------------------------------------------------- + +# fjalar_core_decls.h copies declarations from coregrind headers, which +# tool files must not include. Compile it together with those headers, so +# that the build fails if a Valgrind merge makes the copies wrong. The +# test program is piped to the compiler rather than stored in a .c file, +# because tests/check_headers_and_includes forbids tool .c files from +# including pub_core headers. +BUILT_SOURCES = fjalar_core_decls.stamp +CLEANFILES = fjalar_core_decls.stamp + +fjalar_core_decls.stamp: $(srcdir)/fjalar_core_decls.h \ + $(top_srcdir)/coregrind/pub_core_aspacemgr.h \ + $(top_srcdir)/coregrind/pub_core_mallocfree.h + { echo '#include "fjalar_core_decls.h"'; \ + echo 'enum { fjalar_ar_demangle = VG_AR_DEMANGLE };'; \ + echo '#undef VG_AR_DEMANGLE'; \ + echo '#include "pub_core_aspacemgr.h"'; \ + echo '#include "pub_core_mallocfree.h"'; \ + echo '_Static_assert(fjalar_ar_demangle == VG_AR_DEMANGLE,'; \ + echo ' "VG_AR_DEMANGLE in fjalar_core_decls.h is wrong");'; \ + } | $(CC) $(fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CPPFLAGS) \ + $(fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CFLAGS) \ + -I$(srcdir) -I$(top_srcdir)/coregrind -fsyntax-only -x c - + touch $@ diff --git a/valgrind/fjalar/fjalar_core_decls.h b/valgrind/fjalar/fjalar_core_decls.h new file mode 100644 index 00000000..2161b075 --- /dev/null +++ b/valgrind/fjalar/fjalar_core_decls.h @@ -0,0 +1,40 @@ +/* + This file is part of Fjalar, a dynamic analysis framework for C/C++ + programs. + + Copyright (C) 2007-2026 University of Washington Computer Science & Engineering Department, + Programming Languages and Software Engineering Group + + This program is free software; you can redistribute it and/or + modify it under the terms of the GNU General Public License as + published by the Free Software Foundation; either version 2 of the + License, or (at your option) any later version. +*/ + +/* fjalar_core_decls.h: + +Declarations of the few Valgrind core (non-tool) entities that Fjalar +uses. Tool files must not include coregrind/pub_core_*.h (see +tests/check_headers_and_includes), so these are copied here instead. +The build checks that the copies agree with the originals (see the +fjalar_core_decls.stamp rule in Makefile.am). + +*/ + +#ifndef FJALAR_CORE_DECLS_H +#define FJALAR_CORE_DECLS_H + +#include "pub_tool_basics.h" + +// From coregrind/pub_core_aspacemgr.h. +// mc_check_readable uses this to accept readable memory that Valgrind +// owns, which VG_(am_is_valid_for_client) rejects. +extern Bool VG_(am_is_valid_for_valgrind)(Addr start, SizeT len, UInt prot); + +// From coregrind/pub_core_mallocfree.h. +// Used to release the demanglers' results, which are allocated in the +// VG_AR_DEMANGLE arena. ArenaId is a typedef for Int. +extern void VG_(arena_free)(Int arena, void* ptr); +#define VG_AR_DEMANGLE 3 + +#endif /* FJALAR_CORE_DECLS_H */ diff --git a/valgrind/fjalar/mc_main.c b/valgrind/fjalar/mc_main.c index e2e869d2..6cbc2d8c 100644 --- a/valgrind/fjalar/mc_main.c +++ b/valgrind/fjalar/mc_main.c @@ -69,9 +69,7 @@ #include "kvasir/kvasir_main.h" #include "kvasir/dyncomp_main.h" #include "fjalar_main.h" -// Declared in coregrind/pub_core_aspacemgr.h, which tool files must not -// include (see tests/check_headers_and_includes). -extern Bool VG_(am_is_valid_for_valgrind)(Addr start, SizeT len, UInt prot); +#include "fjalar_core_decls.h" // for VG_(am_is_valid_for_valgrind) #include "mc_include.h" #include "memcheck.h" /* for client requests */ diff --git a/valgrind/fjalar/typedata.c b/valgrind/fjalar/typedata.c index 95cdc27f..aefb8829 100644 --- a/valgrind/fjalar/typedata.c +++ b/valgrind/fjalar/typedata.c @@ -39,12 +39,8 @@ // For name demangling. #include "../coregrind/m_demangle/demangle.h" -// For releasing the demanglers' results. These are declared in -// coregrind/pub_core_mallocfree.h, which tool files must not include (see -// tests/check_headers_and_includes). VG_AR_DEMANGLE must match the value -// there. -extern void VG_(arena_free)(Int arena, void* ptr); -#define VG_AR_DEMANGLE 3 +// For VG_(arena_free), to release the demanglers' results. +#include "fjalar_core_decls.h" // Most of the following information is taken from "DWARF Debugging // Information Format Version 4". Published by the DWARF Debugging From ba9567b06e48d49bfcf391b0e1510ba9e6db4fc8 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Sat, 17 Oct 2026 08:12:24 -0700 Subject: [PATCH 4/5] Run the fjalar_core_decls.h check whenever a fjalar program is built Make each fjalar program depend on fjalar_core_decls.stamp instead of listing the stamp in BUILT_SOURCES, so that targets such as 'make fjalar-amd64-linux' also run the check. Correct the header's path in doc/valgrind-merge.texinfo. --- doc/valgrind-merge.texinfo | 10 +++++----- valgrind/fjalar/Makefile.am | 10 ++++++---- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/doc/valgrind-merge.texinfo b/doc/valgrind-merge.texinfo index 4943ddeb..a691a95f 100644 --- a/doc/valgrind-merge.texinfo +++ b/doc/valgrind-merge.texinfo @@ -497,12 +497,12 @@ changes should be noted in the change logs. Fjalar also uses a few Valgrind core entities that are not part of the tool interface. Tool files may not include the core headers that -declare them, so @file{fjalar/fjalar_core_decls.h} contains copies of -those declarations. The build compiles those copies together with the -core headers, so if the build fails in the rule for +declare them, so @file{valgrind/fjalar/fjalar_core_decls.h} contains +copies of those declarations. The build compiles those copies together +with the core headers, so if the build fails in the rule for @file{fjalar_core_decls.stamp}, then a core declaration has changed. -Update the copy in @file{fjalar/fjalar_core_decls.h} to match the core -header that it names. +Update the copy in @file{valgrind/fjalar/fjalar_core_decls.h} to match +the core header that it names. A somewhat more problematic area are changes in the VEX IR. DynComp makes heavy use of the VEX IR, so any changes in it need to be diff --git a/valgrind/fjalar/Makefile.am b/valgrind/fjalar/Makefile.am index e8e803a9..e19ac0ba 100644 --- a/valgrind/fjalar/Makefile.am +++ b/valgrind/fjalar/Makefile.am @@ -99,7 +99,8 @@ fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CPPFLAGS = \ fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_CFLAGS = $(LTO_CFLAGS) \ $(AM_CFLAGS_@VGCONF_PLATFORM_PRI_CAPS@: -O2=-O0) $(EXTRA_CFLAGS) fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_DEPENDENCIES = \ - $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_PRI_CAPS@) + $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_PRI_CAPS@) \ + fjalar_core_decls.stamp fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LDADD = \ $(TOOL_LDADD_@VGCONF_PLATFORM_PRI_CAPS@) fjalar_@VGCONF_ARCH_PRI@_@VGCONF_OS@_LDFLAGS = \ @@ -119,7 +120,8 @@ fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_CPPFLAGS = \ fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_CFLAGS = $(LTO_CFLAGS) \ $(AM_CFLAGS_@VGCONF_PLATFORM_SEC_CAPS@: -O2=-O0) $(EXTRA_CFLAGS) fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_DEPENDENCIES = \ - $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_SEC_CAPS@) + $(TOOL_DEPENDENCIES_@VGCONF_PLATFORM_SEC_CAPS@) \ + fjalar_core_decls.stamp fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LDADD = \ $(TOOL_LDADD_@VGCONF_PLATFORM_SEC_CAPS@) fjalar_@VGCONF_ARCH_SEC@_@VGCONF_OS@_LDFLAGS = \ @@ -187,8 +189,8 @@ endif # that the build fails if a Valgrind merge makes the copies wrong. The # test program is piped to the compiler rather than stored in a .c file, # because tests/check_headers_and_includes forbids tool .c files from -# including pub_core headers. -BUILT_SOURCES = fjalar_core_decls.stamp +# including pub_core headers. Each fjalar program depends on the stamp, +# so the check runs whenever a program is built. CLEANFILES = fjalar_core_decls.stamp fjalar_core_decls.stamp: $(srcdir)/fjalar_core_decls.h \ From 5b880d38b4e69b7577466de883a7e3bd6369afd5 Mon Sep 17 00:00:00 2001 From: Michael Ernst Date: Sat, 17 Oct 2026 08:51:06 -0700 Subject: [PATCH 5/5] Note in fjalar_core_decls.h which files use it --- doc/valgrind-merge.texinfo | 6 +----- valgrind/fjalar/fjalar_core_decls.h | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/doc/valgrind-merge.texinfo b/doc/valgrind-merge.texinfo index a691a95f..f2b0c839 100644 --- a/doc/valgrind-merge.texinfo +++ b/doc/valgrind-merge.texinfo @@ -495,11 +495,7 @@ changes in Valgrind API/functionality. For the most part Valgrind maintains a relatively stable interface to its tools. Any tool-visible changes should be noted in the change logs. -Fjalar also uses a few Valgrind core entities that are not part of the -tool interface. Tool files may not include the core headers that -declare them, so @file{valgrind/fjalar/fjalar_core_decls.h} contains -copies of those declarations. The build compiles those copies together -with the core headers, so if the build fails in the rule for +If the build fails in the rule for @file{fjalar_core_decls.stamp}, then a core declaration has changed. Update the copy in @file{valgrind/fjalar/fjalar_core_decls.h} to match the core header that it names. diff --git a/valgrind/fjalar/fjalar_core_decls.h b/valgrind/fjalar/fjalar_core_decls.h index 2161b075..25e21852 100644 --- a/valgrind/fjalar/fjalar_core_decls.h +++ b/valgrind/fjalar/fjalar_core_decls.h @@ -14,7 +14,8 @@ /* fjalar_core_decls.h: Declarations of the few Valgrind core (non-tool) entities that Fjalar -uses. Tool files must not include coregrind/pub_core_*.h (see +uses. Fjalar code (e.g., mc_main.c and typedata.c) includes this file +to call those entities. Tool files must not include coregrind/pub_core_*.h (see tests/check_headers_and_includes), so these are copied here instead. The build checks that the copies agree with the originals (see the fjalar_core_decls.stamp rule in Makefile.am).