diff mbox

[v2,6/6] Libsanitizer merge from upstream r250806 (was r249633).

Message ID 562625A8.3030007@partner.samsung.com
State New
Headers show

Commit Message

max Oct. 20, 2015, 11:29 a.m. UTC
In this patch, I'm trying to add a general instruction how to perform 
the merge. This is just a documentation patch, any suggestions and 
opinions are welcome.

Comments

Jakub Jelinek Oct. 20, 2015, 12:01 p.m. UTC | #1
On Tue, Oct 20, 2015 at 02:29:44PM +0300, Maxim Ostapenko wrote:
> In this patch, I'm trying to add a general instruction how to perform the
> merge. This is just a documentation patch, any suggestions and opinions are
> welcome.

I'd add a line that the diff in lib/asan/tests tests since the last
merge to those tests (looking at gcc/testsuite/ChangeLog*, the 2014-05-30 entry
mentions r209283) should be incorporated into the gcc testsuite too (if
possible).

Otherwise, LGTM.

> Index: libsanitizer/HOWTO_MERGE
> ===================================================================
> --- libsanitizer/HOWTO_MERGE	(revision 0)
> +++ libsanitizer/HOWTO_MERGE	(working copy)
> @@ -0,0 +1,26 @@
> +In general, merging process should not be very difficult, but we need to
> +track various ABI changes and GCC-specific patches carefully.  Here is a
> +general list of actions required to perform the merge:
> +
> +- Checkout recent GCC tree.
> +- Run merge.sh script from libsanitizer directory.
> +- Modify Makefile.am files into asan/tsan/lsan/ubsan/sanitizer_common/interception
> +  directories if needed.  In particular, you may need to add new source files
> +  and remove old ones in source files list, add new flags to {C, CXX}FLAGS if
> +  needed and update DEFS with new defined variables.
> +- Apply all needed GCC-specific patches to libsanitizer (note that some of
> +  them might be already included to upstream).
> +- Apply all necessary compiler changes.  Be especially careful here, you must
> +  not break ABI between compiler and library.
> +- Modify configure.ac file if needed (e.g. if you need to add link against new
> +  library for sanitizer lilbs).
> +- Remove unused (deleted by merge) files from all source and include
> +  directories.  Be especially careful with headers, because they aren't listed
> +  in Makefiles explicitly.
> +- Regenerate configure script and all Makefiles by autoreconf.  You should use
> +  exactly the same autotools version as for other GCC directories (current
> +  version is 2.64, https://www.gnu.org/software/automake/faq/autotools-faq.html
> +  for details how to install/use it).
> +- Run regression testing on at least three platforms (e.g. x86-linux-gnu,
> +  x86_64-linux-gnu, aarch64-linux-gnu).
> +- Run {A, UB}San bootstrap on at least three platforms.

	Jakub
Yury Gribov Oct. 21, 2015, 9:17 a.m. UTC | #2
On 10/20/2015 02:29 PM, Maxim Ostapenko wrote:
> In this patch, I'm trying to add a general instruction how to perform
> the merge. This is just a documentation patch, any suggestions and
> opinions are welcome.

Thanks, this should simplify work for other maintainers in future)

Some general remarks:
1) Perhaps use standard markup format for easier reading (i.e. s/^-/*/)?
2) We should suggest to run libabigail to compare ABI of 
libclang_rt-asan and libasan?
3) Perhaps it makes sense to mention that patchset should be split in 
logical pieces?
4) You probably forgot to mention SONAME update.

+- Modify Makefile.am files into 
asan/tsan/lsan/ubsan/sanitizer_common/interception
+  directories if needed.  In particular, you may need to add new source 
files
+  and remove old ones in source files list, add new flags to {C, 
CXX}FLAGS if
+  needed and update DEFS with new defined variables.

1) Could you mention where to look for updates (CMakeLists.txt, etc.).
2) Shouldn't we rerun automake (to update Makefile.in and stuff)?
3) Also add new target platforms (if any).

+- Apply all necessary compiler changes.  Be especially careful here, 
you must
+  not break ABI between compiler and library.

Perhaps mention that for compiler changes one should check commit 
history of e.g. llvm/test/Instrumentation?

+- Remove unused (deleted by merge) files from all source and include
+  directories.

This isn't clear. Doesn't merge.sh handle this?

+- Regenerate configure script and all Makefiles by autoreconf.  You 
should use
+  exactly the same autotools version as for other GCC directories (current
+  version is 2.64, 
https://www.gnu.org/software/automake/faq/autotools-faq.html
+  for details how to install/use it).

Rather than stating explicit version of autotools, perhaps tell where to 
find the current one (e.g. it's written at start of current 
libsanitizer/{Makefile.in,configure}?

+- Run regression testing on at least three platforms (e.g. x86-linux-gnu,
+  x86_64-linux-gnu, aarch64-linux-gnu).

Perhaps ARM as well? We saw a number of platform-specific bugs there.

Best regards,
Yury Gribov
diff mbox

Patch

Index: libsanitizer/HOWTO_MERGE
===================================================================
--- libsanitizer/HOWTO_MERGE	(revision 0)
+++ libsanitizer/HOWTO_MERGE	(working copy)
@@ -0,0 +1,26 @@ 
+In general, merging process should not be very difficult, but we need to
+track various ABI changes and GCC-specific patches carefully.  Here is a
+general list of actions required to perform the merge:
+
+- Checkout recent GCC tree.
+- Run merge.sh script from libsanitizer directory.
+- Modify Makefile.am files into asan/tsan/lsan/ubsan/sanitizer_common/interception
+  directories if needed.  In particular, you may need to add new source files
+  and remove old ones in source files list, add new flags to {C, CXX}FLAGS if
+  needed and update DEFS with new defined variables.
+- Apply all needed GCC-specific patches to libsanitizer (note that some of
+  them might be already included to upstream).
+- Apply all necessary compiler changes.  Be especially careful here, you must
+  not break ABI between compiler and library.
+- Modify configure.ac file if needed (e.g. if you need to add link against new
+  library for sanitizer lilbs).
+- Remove unused (deleted by merge) files from all source and include
+  directories.  Be especially careful with headers, because they aren't listed
+  in Makefiles explicitly.
+- Regenerate configure script and all Makefiles by autoreconf.  You should use
+  exactly the same autotools version as for other GCC directories (current
+  version is 2.64, https://www.gnu.org/software/automake/faq/autotools-faq.html
+  for details how to install/use it).
+- Run regression testing on at least three platforms (e.g. x86-linux-gnu,
+  x86_64-linux-gnu, aarch64-linux-gnu).
+- Run {A, UB}San bootstrap on at least three platforms.