Created attachment 141833 [details]
files in the root directory

Nice new README.freebsd. I assume the new suppression files are correct for freebsd, they are only used for freebsd.

Most new entries in .gitignore look plausible, but I don't know what these are for:

+/vg.cflags
+/vg.config
+/vg.creator
+/vg.creator.user
+/vg.cxxflags
+/vg.files
+/vg.includes

And I don't know whether the new filter entries are relevant (for this commit).

As far as I can tell the Makefile.all.am look correct and should only be used in the freebsd case. Same for Makefile.am and Makefile.tool.am.

Most of the configure.ac changes look correct as far as I can tell they apply to freebsd only. The addition of FLAG_32ON64 looks correct (although the use of gcc9 seems oddly specific). Adding -Wexpansion-to-defined is probably a good thing.

I don't think adding -Werror when pie support is detected is correct:

-CFLAGS="-no-pie"
+CFLAGS="-no-pie -Werror"

The --image-base and/or -Ttext-segment changes look correct.

If -pthread is needed for AC_CHECK_FUNCS doesn't it need to be set in some build flag?

It looks like some of the AC_CONFIG_FILES should go into some later patch, which will introduce these files. Those generate errors during ./autogen.sh making this patch untestable on its own.

I'll remove the vg.* files in .gitignore. That's for my Qt Creator project. I forgot to remove those lines.

The path for gcc9. I need to check to see what gets installed on FreeSD 13 and 14, and then to see how I can get the path based on the GCC version. For most users this is not much of an issue as they will be using clang.

I'll check and update shortly on the need for the changes related to -pie and -pthread. If they really are necessary I'll make them conditional.

When I made these patches, I didn't make sure that they apply separately. I'll take a look to see what the memcheck/tests/Makefile.am dependencies are.

> I don't think adding -Werror when pie support is detected is correct

I was incorrect here, this is for the flag detection, adding -Werror is correct here and probably helps clang because it just silently accepts any flag gcc accepts, even when not implemented, except when -Werror is given.

I still do like to understand when the -pthread addition is necessary for AC_CHECK_FUNCS. We might want to separate the functions for which this is necessary.

Created attachment 141930 [details]
freebsd2 patch

This is the variant of the patch that works for me on fedora x86_64. It removed some .gitignore entries (specifically the various filter_* entries which I believe are too broad), the extra -pthread addition (which I like to understand better) and the addition of the extra CONFIG files (which should be added to later patches that add the files themselves).

I need to keep the filter ignore lines as I've changed these files to be automake generated. Doing this allows me to use gsed without having to think if the sed expressions are compatible or not between GNU sed and FreeBSD sed. The changed filter files are in the separate pathes for drd. memcheck etc.

The AC_CONFIG files are indeed spread amongst later patches.

It looks like the -pthread lines are not necessary.

In README.freebsd,  I should replace my github address with the valgrind bugzilla as the place to go for feedback.

FWIW while reviewing I noticed some EOL whitespace (because my editor highlighted it), e.g. in README.freebsd.

Thanks Ed, I'll probably go through "git diff origin" and remove any offending red whitespace that I've added.

I've just updated the README and also modified the feedback section.

Just been through all of the files and removed most of the trailing whitespace. Probably missed a bit, but most is gone.

Created attachment 142183 [details]
files in the root directory

configure.ac is still not quite right

Created attachment 142194 [details]
files in the root directory

Added some missing changes in coregrind Makefile.am

Code committed in
commit e2583c02a5638fcbe7b3d693f7127ebde5d7c359