You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sam James <sa...@gentoo.org> on 2025/02/26 10:09:08 UTC
Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Hi!
When subversion-1.14.5 is built with upcoming GCC 15 with -O3 -mavx (or
-O2 -mavx -fno-vect-cost-model), it crashes in some cases in
svn_eol__find_eol_start.
We found `svn checkout https://svn.code.sf.net/p/sdcc/code/trunk/sdcc`
was a reliable reproducer, as was `svn cleanup sdcc` once the first
command crashed. (The svn testsuite passes, though.)
Full backtrace:
```
Thread 1 (Thread 0x7fb5f533d7c0 (LWP 418455)):
#0 0x00007fb5f5be084d in svn_eol__find_eol_start (buf=<optimized out>, buf@entry=0x564804d4a48b " bug3379723.c\n*/\n\n#include <testfwk.h>\n\n#if !defined(PORT_HOST) && !defined(__SDCC_hc08) && !defined(__SDCC_s08) && !defined(__SDCC_mos6502) && !defined(__SDCC_mos65c02) && !defined(__SDCC_sm83) && "..., len=<optimized out>, len@entry=838) at subversion/libsvn_subr/eol.c:43
#1 0x00007fb5f5c0d2ca in translate_chunk (dst=dst@entry=0x564804d56840, b=0x56480490dec0, buf=<optimized out>, buflen=<optimized out>, pool=<optimized out>) at subversion/libsvn_subr/subst.c:1152
#2 0x00007fb5f5c0e91d in translated_stream_read (baton=0x56480490de20, buffer=0x564804d4f498 "/*\n bug2686159.c\n */\n\n#include <testfwk.h>\n\n#ifdef __SDCC_pic16\n# define ADDRESS(x) (0x02 ## x)\n#elif defined(__SDCC_pic14)\n# define ADDRESS(x) (0x01A0 + 0x ## x)\n#elif defined(__SDCC_stm8)\n# define"..., len=0x7ffe9abfae10) at subversion/libsvn_subr/subst.c:1297
#3 0x00007fb5f5c08647 in svn_stream_copy3 (from=0x56480490de60, to=0x564804d49548, cancel_func=cancel_func@entry=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=cancel_baton@entry=0x0, scratch_pool=scratch_pool@entry=0x56480490c6f8) at subversion/libsvn_subr/stream.c:423
#4 0x00007fb5f5d01d45 in run_file_install (wqb=0x7ffe9abfaf90, db=0x56480464e268, work_item=<optimized out>, wri_abspath=0x5648049de410 "/home/kostadin/sdcc/support/regression/tests", cancel_func=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=0x0, scratch_pool=0x56480490c6f8) at subversion/libsvn_wc/workqueue.c:580
#5 0x00007fb5f5d02fdd in dispatch_work_item (wqb=<optimized out>, db=<optimized out>, wri_abspath=<optimized out>, work_item=<optimized out>, cancel_func=<optimized out>, cancel_baton=<optimized out>, scratch_pool=<optimized out>) at subversion/libsvn_wc/workqueue.c:1441
#6 svn_wc__wq_run (db=0x56480464e268, wri_abspath=0x5648049de410 "/home/kostadin/sdcc/support/regression/tests", cancel_func=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=0x0, scratch_pool=scratch_pool@entry=0x5648049de2a8) at subversion/libsvn_wc/workqueue.c:1546
#7 0x00007fb5f5cc4f0c in close_directory (dir_baton=0x5648049de320, pool=<optimized out>) at subversion/libsvn_wc/update_editor.c:2819
#8 0x00007fb5f55c89b3 in maybe_close_dir (dir=<optimized out>) at subversion/libsvn_ra_serf/update.c:738
#9 0x00007fb5f55ce0fc in handle_response_cb (request=0x5648048aee18, response=<optimized out>, baton=0x5648048a0ff0, response_pool=0x564804d46468) at subversion/libsvn_ra_serf/util.c:1540
#10 0x00007fb5f5393f8e in serf.process_connection () from /usr/lib64/libserf-1.so.1
#11 0x00007fb5f5394815 in serf_event_trigger () from /usr/lib64/libserf-1.so.1
#12 0x00007fb5f5394989 in serf_context_run () from /usr/lib64/libserf-1.so.1
#13 0x00007fb5f55cd7b1 in svn_ra_serf__context_run (sess=0x564804679c60, waittime_left=0x7ffe9abfb3a0, scratch_pool=0x5648047128b8) at subversion/libsvn_ra_serf/util.c:913
#14 svn_ra_serf__context_run (sess=0x564804679c60, waittime_left=0x7ffe9abfb3a0, scratch_pool=0x5648047128b8) at subversion/libsvn_ra_serf/util.c:902
#15 0x00007fb5f55c998d in process_editor_report (ctx=0x564804661b28, handler=0x564804690a08, scratch_pool=0x564804690878) at subversion/libsvn_ra_serf/update.c:2429
#16 finish_report (report_baton=0x564804661b28, pool=<optimized out>) at subversion/libsvn_ra_serf/update.c:2504
#17 0x00007fb5f5c868d0 in svn_wc_crawl_revisions5 (wc_ctx=<optimized out>, local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc", reporter=0x7fb5f55df960 <ra_serf_reporter>, report_baton=0x564804661b28, restore_files=restore_files@entry=1, depth=depth@entry=svn_depth_unknown, honor_depth_exclude=1, depth_compatibility_trick=0, use_commit_times=0, cancel_func=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=0x0, notify_func=0x5647fc07f070 <svn_cl__check_externals_failed_notify_wrapper>, notify_baton=0x7ffe9abfb9e0, scratch_pool=0x56480465f788) at subversion/libsvn_wc/adm_crawler.c:859
#18 0x00007fb5f5dbb741 in update_internal (result_rev=result_rev@entry=0x0, timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, conflicted_paths=conflicted_paths@entry=0x0, ra_session_p=<optimized out>, local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc", anchor_abspath=anchor_abspath@entry=0x564804661350 "/home/kostadin/sdcc", revision=0x7ffe9abfb790, depth=svn_depth_unknown, depth_is_sticky=0, ignore_externals=0, allow_unver_obstructions=0, adds_as_modification=1, notify_summary=1, ctx=0x56480464e180, result_pool=0x56480465f788, scratch_pool=0x56480465f788) at subversion/libsvn_client/update.c:560
#19 0x00007fb5f5dbbc8c in svn_client__update_internal (result_rev=result_rev@entry=0x0, timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc", revision=revision@entry=0x7ffe9abfb870, depth=depth@entry=svn_depth_unknown, depth_is_sticky=depth_is_sticky@entry=1, ignore_externals=0, allow_unver_obstructions=0, adds_as_modification=1, make_parents=0, innerupdate=0, ra_session=<optimized out>, ctx=0x56480464e180, pool=0x56480465f788) at subversion/libsvn_client/update.c:696
#20 0x00007fb5f5d5336d in svn_client__checkout_internal (result_rev=result_rev@entry=0x0, timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, url=url@entry=0x56480465e1f8 "https://svn.code.sf.net/p/sdcc/code/trunk/sdcc", local_abspath=0x56480465f8a0 "/home/kostadin/sdcc", peg_revision=peg_revision@entry=0x7ffe9abfb9d0, revision=revision@entry=0x7ffe9abfb9c0, depth=svn_depth_unknown, ignore_externals=0, allow_unver_obstructions=0, ra_session=<optimized out>, ctx=0x56480464e180, scratch_pool=0x56480465f788) at subversion/libsvn_client/checkout.c:189
#21 0x00007fb5f5d53594 in svn_client_checkout3 (result_rev=result_rev@entry=0x0, URL=0x56480465e1f8 "https://svn.code.sf.net/p/sdcc/code/trunk/sdcc", path=<optimized out>, peg_revision=peg_revision@entry=0x7ffe9abfb9d0, revision=revision@entry=0x7ffe9abfb9c0, depth=svn_depth_unknown, ignore_externals=0, allow_unver_obstructions=0, ctx=0x56480464e180, pool=0x56480465f788) at subversion/libsvn_client/checkout.c:218
#22 0x00005647fc06bef2 in svn_cl__checkout (os=<optimized out>, baton=<optimized out>, pool=<optimized out>) at subversion/svn/checkout-cmd.c:168
#23 0x00005647fc08b7eb in sub_main (exit_code=exit_code@entry=0x7ffe9abfbe34, argc=argc@entry=3, cmdline_argv=cmdline_argv@entry=0x7ffe9abfbf68, pool=pool@entry=0x56480464c638) at subversion/svn/svn.c:3263
#24 0x00005647fc06983f in main (argc=3, argv=0x7ffe9abfbf68) at
subversion/svn/svn.c:3348
```
The unaligned access itself controlled by SVN_UNALIGNED_ACCESS_IS_OK in
svn_eol__find_eol_start AFAIK isn't wrong, just the forming and use of
an unaligned pointer for (apr_)uintptr_t.
-fsanitize=undefined (or -fsanitize=alignment, take your pick; it's a
subset) reports:
```
subversion/libsvn_subr/eol.c:43:21: runtime error: load of misaligned address 0x56302447f98b for type 'const apr_uintptr_t', which requires 8 byte alignment
0x56302447f98b: note: pointer points here
00 2f 2a 0a 20 20 20 62 75 67 33 33 37 39 37 32 33 2e 63 0a 2a 2f 0a 0a 23 69 6e 63 6c 75 64 65
^
```
It was originally reported downstream in Gentoo at
https://bugs.gentoo.org/950271 and (erroneously by me later) to GCC at
https://gcc.gnu.org/PR119016.
thanks,
sam
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Sam James <sa...@gentoo.org>.
Daniel Sahlberg <da...@gmail.com> writes:
> Den ons 26 feb. 2025 kl 12:33 skrev Sam James <sa...@gentoo.org>:
>
> Hi!
>
> Hi,
Hi Daniel,
>
> Thanks for the detailed report!
And thank you for the quick response (and Stefan too).
>
> When subversion-1.14.5 is built with upcoming GCC 15 with -O3 -mavx (or
> -O2 -mavx -fno-vect-cost-model), it crashes in some cases in
> svn_eol__find_eol_start.
>
> We found `svn checkout https://svn.code.sf.net/p/sdcc/code/trunk/sdcc`
> was a reliable reproducer, as was `svn cleanup sdcc` once the first
> command crashed. (The svn testsuite passes, though.)
>
> Full backtrace:
> ```
> Thread 1 (Thread 0x7fb5f533d7c0 (LWP 418455)):
> #0 0x00007fb5f5be084d in svn_eol__find_eol_start (buf=<optimized out>, buf@entry=0x564804d4a48b "
> bug3379723.c\n*/\n\n#include <testfwk.h>\n\n#if !defined(PORT_HOST) && !defined(__SDCC_hc08) && !defined(__SDCC_s08)
> && !defined(__SDCC_mos6502) && !defined(__SDCC_mos65c02) && !defined(__SDCC_sm83) && "..., len=<optimized out>,
> len@entry=838) at subversion/libsvn_subr/eol.c:43
> #1 0x00007fb5f5c0d2ca in translate_chunk (dst=dst@entry=0x564804d56840, b=0x56480490dec0, buf=<optimized out>,
> buflen=<optimized out>, pool=<optimized out>) at subversion/libsvn_subr/subst.c:1152
> #2 0x00007fb5f5c0e91d in translated_stream_read (baton=0x56480490de20, buffer=0x564804d4f498 "/*\n bug2686159.c\n
> */\n\n#include <testfwk.h>\n\n#ifdef __SDCC_pic16\n# define ADDRESS(x) (0x02 ## x)\n#elif defined(__SDCC_pic14)\n#
> define ADDRESS(x) (0x01A0 + 0x ## x)\n#elif defined(__SDCC_stm8)\n# define"..., len=0x7ffe9abfae10) at
> subversion/libsvn_subr/subst.c:1297
> #3 0x00007fb5f5c08647 in svn_stream_copy3 (from=0x56480490de60, to=0x564804d49548,
> cancel_func=cancel_func@entry=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=cancel_baton@entry=0x0,
> scratch_pool=scratch_pool@entry=0x56480490c6f8) at subversion/libsvn_subr/stream.c:423
> #4 0x00007fb5f5d01d45 in run_file_install (wqb=0x7ffe9abfaf90, db=0x56480464e268, work_item=<optimized out>,
> wri_abspath=0x5648049de410 "/home/kostadin/sdcc/support/regression/tests", cancel_func=0x7fb5f5bcffb0 <check_cancel>,
> cancel_baton=0x0, scratch_pool=0x56480490c6f8) at subversion/libsvn_wc/workqueue.c:580
> #5 0x00007fb5f5d02fdd in dispatch_work_item (wqb=<optimized out>, db=<optimized out>, wri_abspath=<optimized out>,
> work_item=<optimized out>, cancel_func=<optimized out>, cancel_baton=<optimized out>, scratch_pool=<optimized out>)
> at subversion/libsvn_wc/workqueue.c:1441
> #6 svn_wc__wq_run (db=0x56480464e268, wri_abspath=0x5648049de410 "/home/kostadin/sdcc/support/regression/tests",
> cancel_func=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=0x0, scratch_pool=scratch_pool@entry=0x5648049de2a8) at
> subversion/libsvn_wc/workqueue.c:1546
> #7 0x00007fb5f5cc4f0c in close_directory (dir_baton=0x5648049de320, pool=<optimized out>) at
> subversion/libsvn_wc/update_editor.c:2819
> #8 0x00007fb5f55c89b3 in maybe_close_dir (dir=<optimized out>) at subversion/libsvn_ra_serf/update.c:738
> #9 0x00007fb5f55ce0fc in handle_response_cb (request=0x5648048aee18, response=<optimized out>, baton=0x5648048a0ff0,
> response_pool=0x564804d46468) at subversion/libsvn_ra_serf/util.c:1540
> #10 0x00007fb5f5393f8e in serf.process_connection () from /usr/lib64/libserf-1.so.1
> #11 0x00007fb5f5394815 in serf_event_trigger () from /usr/lib64/libserf-1.so.1
> #12 0x00007fb5f5394989 in serf_context_run () from /usr/lib64/libserf-1.so.1
> #13 0x00007fb5f55cd7b1 in svn_ra_serf__context_run (sess=0x564804679c60, waittime_left=0x7ffe9abfb3a0,
> scratch_pool=0x5648047128b8) at subversion/libsvn_ra_serf/util.c:913
> #14 svn_ra_serf__context_run (sess=0x564804679c60, waittime_left=0x7ffe9abfb3a0, scratch_pool=0x5648047128b8) at
> subversion/libsvn_ra_serf/util.c:902
> #15 0x00007fb5f55c998d in process_editor_report (ctx=0x564804661b28, handler=0x564804690a08,
> scratch_pool=0x564804690878) at subversion/libsvn_ra_serf/update.c:2429
> #16 finish_report (report_baton=0x564804661b28, pool=<optimized out>) at subversion/libsvn_ra_serf/update.c:2504
> #17 0x00007fb5f5c868d0 in svn_wc_crawl_revisions5 (wc_ctx=<optimized out>,
> local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc", reporter=0x7fb5f55df960 <ra_serf_reporter>,
> report_baton=0x564804661b28, restore_files=restore_files@entry=1, depth=depth@entry=svn_depth_unknown,
> honor_depth_exclude=1, depth_compatibility_trick=0, use_commit_times=0, cancel_func=0x7fb5f5bcffb0 <check_cancel>,
> cancel_baton=0x0, notify_func=0x5647fc07f070 <svn_cl__check_externals_failed_notify_wrapper>,
> notify_baton=0x7ffe9abfb9e0, scratch_pool=0x56480465f788) at subversion/libsvn_wc/adm_crawler.c:859
> #18 0x00007fb5f5dbb741 in update_internal (result_rev=result_rev@entry=0x0,
> timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, conflicted_paths=conflicted_paths@entry=0x0,
> ra_session_p=<optimized out>, local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc",
> anchor_abspath=anchor_abspath@entry=0x564804661350 "/home/kostadin/sdcc", revision=0x7ffe9abfb790,
> depth=svn_depth_unknown, depth_is_sticky=0, ignore_externals=0, allow_unver_obstructions=0, adds_as_modification=1,
> notify_summary=1, ctx=0x56480464e180, result_pool=0x56480465f788, scratch_pool=0x56480465f788) at
> subversion/libsvn_client/update.c:560
> #19 0x00007fb5f5dbbc8c in svn_client__update_internal (result_rev=result_rev@entry=0x0,
> timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, local_abspath=local_abspath@entry=0x56480465f8a0
> "/home/kostadin/sdcc", revision=revision@entry=0x7ffe9abfb870, depth=depth@entry=svn_depth_unknown,
> depth_is_sticky=depth_is_sticky@entry=1, ignore_externals=0, allow_unver_obstructions=0, adds_as_modification=1,
> make_parents=0, innerupdate=0, ra_session=<optimized out>, ctx=0x56480464e180, pool=0x56480465f788) at
> subversion/libsvn_client/update.c:696
> #20 0x00007fb5f5d5336d in svn_client__checkout_internal (result_rev=result_rev@entry=0x0,
> timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c, url=url@entry=0x56480465e1f8
> "https://svn.code.sf.net/p/sdcc/code/trunk/sdcc", local_abspath=0x56480465f8a0 "/home/kostadin/sdcc",
> peg_revision=peg_revision@entry=0x7ffe9abfb9d0, revision=revision@entry=0x7ffe9abfb9c0, depth=svn_depth_unknown,
> ignore_externals=0, allow_unver_obstructions=0, ra_session=<optimized out>, ctx=0x56480464e180,
> scratch_pool=0x56480465f788) at subversion/libsvn_client/checkout.c:189
> #21 0x00007fb5f5d53594 in svn_client_checkout3 (result_rev=result_rev@entry=0x0, URL=0x56480465e1f8
> "https://svn.code.sf.net/p/sdcc/code/trunk/sdcc", path=<optimized out>,
> peg_revision=peg_revision@entry=0x7ffe9abfb9d0, revision=revision@entry=0x7ffe9abfb9c0, depth=svn_depth_unknown,
> ignore_externals=0, allow_unver_obstructions=0, ctx=0x56480464e180, pool=0x56480465f788) at
> subversion/libsvn_client/checkout.c:218
> #22 0x00005647fc06bef2 in svn_cl__checkout (os=<optimized out>, baton=<optimized out>, pool=<optimized out>) at
> subversion/svn/checkout-cmd.c:168
> #23 0x00005647fc08b7eb in sub_main (exit_code=exit_code@entry=0x7ffe9abfbe34, argc=argc@entry=3,
> cmdline_argv=cmdline_argv@entry=0x7ffe9abfbf68, pool=pool@entry=0x56480464c638) at subversion/svn/svn.c:3263
> #24 0x00005647fc06983f in main (argc=3, argv=0x7ffe9abfbf68) at
> subversion/svn/svn.c:3348
> ```
>
> The unaligned access itself controlled by SVN_UNALIGNED_ACCESS_IS_OK in
> svn_eol__find_eol_start AFAIK isn't wrong, just the forming and use of
> an unaligned pointer for (apr_)uintptr_t.
>
> Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was set to 0 (so the char by char loop after #endif
> is used instead), the code would run just fine. Can you confirm that?
>
Confirmed. And I've also checked it vectorises (though not benchmarked
it).
> SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to change that for GCC15.
>
> #ifndef SVN_UNALIGNED_ACCESS_IS_OK
> # if defined(_M_IX86) || defined(i386) \
> || defined(_M_X64) || defined(__x86_64) \
> || defined(__powerpc__) || defined(__ppc__)
> # define SVN_UNALIGNED_ACCESS_IS_OK 1
> # else
> # define SVN_UNALIGNED_ACCESS_IS_OK 0
> # endif
> #endif
>
> -fsanitize=undefined (or -fsanitize=alignment, take your pick; it's a
> subset) reports:
> ```
> subversion/libsvn_subr/eol.c:43:21: runtime error: load of misaligned address 0x56302447f98b for type 'const
> apr_uintptr_t', which requires 8 byte alignment
> 0x56302447f98b: note: pointer points here
> 00 2f 2a 0a 20 20 20 62 75 67 33 33 37 39 37 32 33 2e 63 0a 2a 2f 0a 0a 23 69 6e 63 6c 75 64 65
> ^
> ```
>
thanks,
sam
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Sam James <sa...@gentoo.org>.
Daniel Sahlberg <da...@gmail.com> writes:
> Den ons 26 feb. 2025 kl 14:45 skrev Daniel Sahlberg <da...@gmail.com>:
>
> Den ons 26 feb. 2025 kl 14:30 skrev Stefan Sperling <st...@apache.org>:
>
> On Wed, Feb 26, 2025 at 01:24:22PM +0100, Daniel Sahlberg wrote:
> > Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was
> > set to 0 (so the char by char loop after #endif is used instead), the code
> > would run just fine. Can you confirm that?
> >
> > SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to
> > change that for GCC15.
>
> Unaligned access is undefined behaviour (UB). Compilers are now taking
> more liberties with undefined behaviour than they used to. So we need
> to be very careful in our assumptions about UB.
>
> Our code which is guarded by SVN_UNALIGNED_ACCESS_IS_OK might become
> unsafe when built with newer compilers on any platform. This code's
> behaviour is no longer under our control, but the compiler's s control.
> svn_eol__find_eol_start() might crash, become a no-op, or have unknown
> side-effects resulting in annoyances or even CVEs.
>
> At the very least, we should have this macro off by default and require
> it to be enabled manually. For best safety, the code should be removed.
> I understand that removing this code will incur a performance cost.
> The code was backed up by elaborate performance testing when it was
> written years ago.
>
> But what modern compilers make of UB will likely keep changing for the
> worse (or better, depending on which side of the UB-fence you are on),
> violating assumptions about platform behaviour in the macro definition:
>
> > #ifndef SVN_UNALIGNED_ACCESS_IS_OK
> > # if defined(_M_IX86) || defined(i386) \
> > || defined(_M_X64) || defined(__x86_64) \
> > || defined(__powerpc__) || defined(__ppc__)
> > # define SVN_UNALIGNED_ACCESS_IS_OK 1
> > # else
> > # define SVN_UNALIGNED_ACCESS_IS_OK 0
> > # endif
> > #endif
>
> I have seen contrived programs being translated into nothing on x86 when
> built with a relatively recent version of clang (16) at optimization levels
> of -O2 and up. For example, the following program will literally have its
> entire main() function elided:
>
> [[[
> #include <float.h>
> int main(void) {
> double v = DBL_MAX;
> int v2 = v;
> if (v2 != v) return 0;
> return 1;
> }
> ]]]
> (courtesy of https://bsd.network/@kristapsdz/113870782376321887)
>
> On OpenBSD/amd64, with clang -O2 this program compiles to 3 instructions,
> an empty function. The resulting executable just crashes when run.
> [[[
> (gdb) disassemble main
> 0x0000000000001950 <+0>: endbr64
> 0x0000000000001954 <+4>: push %rbp
> 0x0000000000001955 <+5>: mov %rsp,%rbp
> ]]]
>
> At -01 there are 4 lines (still crashes). Without optimization (-O0)
> there are 38 lines of assembly instead of 3, and the program exits
> with status 0.
>
> Granted, what I have seen are contrived cases. But they resulted
> from someone looking into problems while doing development in C.
> As a C developer, this scares me enough to avoid UB at all costs
> because I cannot trust the compiler to warn about my mistakes
> which introduce UB. Rather, the compiler might decide to elide
> the code and change the intended meaning of the program.
>
> Thanks for the detailed explanation. It seems really ugly.
>
> Would add a separate for-loop before "one machine word at a time", iterating until buf is aligned (maximum sizeof
> (void*)-1 times) help here?
Good news!
Tamar Christina who's been working on the vectoriser in GCC pointed out
that if you just keep the byteloop at the end, GCC 15 at least (with
the right -march and -On) should ensure alignment by peeling and
then vectorise the rest.
> [...]
>
> Any idea of the performance gain on SVN_UNALIGNED_ACCESS_IS_OK on modern hardware/modern compilers?
>
See above, but in my experience, these #ifdef'd unaligned paths are
usually superfluous with modern compilers or buggy, or both. I wouldn't
lose a night's sleep over just dropping it.
> Cheers,
> Daniel
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Branko Čibej <br...@apache.org>.
On 28. 2. 25 15:23, Daniel Sahlberg wrote:
> Den fre 28 feb. 2025 kl 15:16 skrev Stefan Sperling <st...@apache.org>:
>
> On Fri, Feb 28, 2025 at 02:14:34PM +0100, Branko Čibej wrote:
> > This is probably the minimal patch to disable this tweak.
>
> Thanks, your diff looks good to me.
>
> > There's a bunch of
> > code that needs to be replaced or removed, though.
>
> These bits of code are easy to spot and remove.
> Here is a diff for that:
>
>
> Thanks to both Branko and Stefan. Both patches look good to me at a
> quick glance. I was about to do something similar myself.
r1924100
> I assume the optimisation we can get from the vector instructions are
> a lot bigger than the "hand optimised" C code so it should be good to
> go and anyone performance critical will compile with vector instructions.
I'd be surprised if that wasn't the case. Anyway, at least in one case
that I looked at (diff_file.c), the "optimization" adds another
iteration over the in-memory data, and changes the semantics of the
enclosing function. I have a hard time understanding how that makes
anything faster.
-- Brane
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 28 feb. 2025 kl 15:16 skrev Stefan Sperling <st...@apache.org>:
> On Fri, Feb 28, 2025 at 02:14:34PM +0100, Branko Čibej wrote:
> > This is probably the minimal patch to disable this tweak.
>
> Thanks, your diff looks good to me.
>
> > There's a bunch of
> > code that needs to be replaced or removed, though.
>
> These bits of code are easy to spot and remove.
> Here is a diff for that:
>
Thanks to both Branko and Stefan. Both patches look good to me at a quick
glance. I was about to do something similar myself.
I assume the optimisation we can get from the vector instructions are a lot
bigger than the "hand optimised" C code so it should be good to go and
anyone performance critical will compile with vector instructions.
Cheers,
Daniel
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Stefan Sperling <st...@apache.org>.
On Fri, Feb 28, 2025 at 02:14:34PM +0100, Branko Čibej wrote:
> This is probably the minimal patch to disable this tweak.
Thanks, your diff looks good to me.
> There's a bunch of
> code that needs to be replaced or removed, though.
These bits of code are easy to spot and remove.
Here is a diff for that:
[[[
Remove optmized unaligned-access code paths.
The newer compilers using vector instructions these manual optimizations can
lead to build errors and do not provide benefits over compiler optimizations.
* subversion/libsvn_diff/diff_file.c
(contains_eol): Remove.
(find_identical_prefix, find_identical_suffix): Remove code under
#ifdef SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_fs_fs/tree.c
(hash_func): Remove code under #ifdef SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_fs_x/dag_cache.c
(cache_lookup): Remove code under #ifdef SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_fs_x/string_table.c
(copy_masks): Remove.
(table_copy_string): Remove code under #ifdef SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_subr/eol.c
(svn_eol__find_eol_start): Remove code under #ifdef
SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_subr/hash.c
(hashfunc_compatible): Remove code under #ifdef SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_subr/string.c
(svn_cstring__match_length): Remove code under #ifdef
SVN_UNALIGNED_ACCESS_IS_OK.
* subversion/libsvn_subr/utf_validate.c
(first_non_fsm_start_char): Remove code under #ifdef
SVN_UNALIGNED_ACCESS_IS_OK.
Reported by: Sam James from gentoo
]]]
Index: subversion/libsvn_diff/diff_file.c
===================================================================
--- subversion/libsvn_diff/diff_file.c (revision 1924098)
+++ subversion/libsvn_diff/diff_file.c (working copy)
@@ -355,23 +355,6 @@ is_one_at_eof(struct file_info file[], apr_size_t
return FALSE;
}
-/* Quickly determine whether there is a eol char in CHUNK.
- * (mainly copy-n-paste from eol.c#svn_eol__find_eol_start).
- */
-
-#if SVN_UNALIGNED_ACCESS_IS_OK
-static svn_boolean_t contains_eol(apr_uintptr_t chunk)
-{
- apr_uintptr_t r_test = chunk ^ SVN__R_MASK;
- apr_uintptr_t n_test = chunk ^ SVN__N_MASK;
-
- r_test |= (r_test & SVN__LOWER_7BITS_SET) + SVN__LOWER_7BITS_SET;
- n_test |= (n_test & SVN__LOWER_7BITS_SET) + SVN__LOWER_7BITS_SET;
-
- return (r_test & n_test & SVN__BIT_7_SET) != SVN__BIT_7_SET;
-}
-#endif
-
/* Find the prefix which is identical between all elements of the FILE array.
* Return the number of prefix lines in PREFIX_LINES. REACHED_ONE_EOF will be
* set to TRUE if one of the FILEs reached its end while scanning prefix,
@@ -396,10 +379,6 @@ find_identical_prefix(svn_boolean_t *reached_one_e
is_match = is_match && *file[0].curp == *file[i].curp;
while (is_match)
{
-#if SVN_UNALIGNED_ACCESS_IS_OK
- apr_ssize_t max_delta, delta;
-#endif /* SVN_UNALIGNED_ACCESS_IS_OK */
-
/* ### TODO: see if we can take advantage of
diff options like ignore_eol_style or ignore_space. */
/* check for eol, and count */
@@ -419,53 +398,6 @@ find_identical_prefix(svn_boolean_t *reached_one_e
INCREMENT_POINTERS(file, file_len, pool);
-#if SVN_UNALIGNED_ACCESS_IS_OK
-
- /* Try to advance as far as possible with machine-word granularity.
- * Determine how far we may advance with chunky ops without reaching
- * endp for any of the files.
- * Signedness is important here if curp gets close to endp.
- */
- max_delta = file[0].endp - file[0].curp - sizeof(apr_uintptr_t);
- for (i = 1; i < file_len; i++)
- {
- delta = file[i].endp - file[i].curp - sizeof(apr_uintptr_t);
- if (delta < max_delta)
- max_delta = delta;
- }
-
- is_match = TRUE;
- for (delta = 0; delta < max_delta; delta += sizeof(apr_uintptr_t))
- {
- apr_uintptr_t chunk = *(const apr_uintptr_t *)(file[0].curp + delta);
- if (contains_eol(chunk))
- break;
-
- for (i = 1; i < file_len; i++)
- if (chunk != *(const apr_uintptr_t *)(file[i].curp + delta))
- {
- is_match = FALSE;
- break;
- }
-
- if (! is_match)
- break;
- }
-
- if (delta /* > 0*/)
- {
- /* We either found a mismatch or an EOL at or shortly behind curp+delta
- * or we cannot proceed with chunky ops without exceeding endp.
- * In any way, everything up to curp + delta is equal and not an EOL.
- */
- for (i = 0; i < file_len; i++)
- file[i].curp += delta;
-
- /* Skipped data without EOL markers, so last char was not a CR. */
- had_cr = FALSE;
- }
-#endif
-
*reached_one_eof = is_one_at_eof(file, file_len);
if (*reached_one_eof)
break;
@@ -611,11 +543,6 @@ find_identical_suffix(apr_off_t *suffix_lines, str
while (is_match)
{
svn_boolean_t reached_prefix;
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* Initialize the minimum pointer positions. */
- const char *min_curp[4];
- svn_boolean_t can_read_word;
-#endif /* SVN_UNALIGNED_ACCESS_IS_OK */
/* ### TODO: see if we can take advantage of
diff options like ignore_eol_style or ignore_space. */
@@ -636,60 +563,6 @@ find_identical_suffix(apr_off_t *suffix_lines, str
DECREMENT_POINTERS(file_for_suffix, file_len, pool);
-#if SVN_UNALIGNED_ACCESS_IS_OK
- for (i = 0; i < file_len; i++)
- min_curp[i] = file_for_suffix[i].buffer;
-
- /* If we are in the same chunk that contains the last part of the common
- prefix, use the min_curp[0] pointer to make sure we don't get a
- suffix that overlaps the already determined common prefix. */
- if (file_for_suffix[0].chunk == suffix_min_chunk0)
- min_curp[0] += suffix_min_offset0;
-
- /* Scan quickly by reading with machine-word granularity. */
- for (i = 0, can_read_word = TRUE; can_read_word && i < file_len; i++)
- can_read_word = ((file_for_suffix[i].curp + 1 - sizeof(apr_uintptr_t))
- > min_curp[i]);
-
- while (can_read_word)
- {
- apr_uintptr_t chunk;
-
- /* For each file curp is positioned at the current byte, but we
- want to examine the current byte and the ones before the current
- location as one machine word. */
-
- chunk = *(const apr_uintptr_t *)(file_for_suffix[0].curp + 1
- - sizeof(apr_uintptr_t));
- if (contains_eol(chunk))
- break;
-
- for (i = 1, is_match = TRUE; is_match && i < file_len; i++)
- is_match = (chunk
- == *(const apr_uintptr_t *)
- (file_for_suffix[i].curp + 1
- - sizeof(apr_uintptr_t)));
-
- if (! is_match)
- break;
-
- for (i = 0; i < file_len; i++)
- {
- file_for_suffix[i].curp -= sizeof(apr_uintptr_t);
- can_read_word = can_read_word
- && ( (file_for_suffix[i].curp + 1
- - sizeof(apr_uintptr_t))
- > min_curp[i]);
- }
-
- /* We skipped some bytes, so there are no closing EOLs */
- had_nl = FALSE;
- }
-
- /* The > min_curp[i] check leaves at least one final byte for checking
- in the non block optimized case below. */
-#endif
-
reached_prefix = file_for_suffix[0].chunk == suffix_min_chunk0
&& (file_for_suffix[0].curp - file_for_suffix[0].buffer)
== suffix_min_offset0;
Index: subversion/libsvn_fs_fs/tree.c
===================================================================
--- subversion/libsvn_fs_fs/tree.c (revision 1924098)
+++ subversion/libsvn_fs_fs/tree.c (working copy)
@@ -220,11 +220,6 @@ hash_func(svn_revnum_t revision,
apr_size_t i;
apr_uint32_t hash_value = (apr_uint32_t)revision;
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* "randomizing" / distributing factor used in our hash function */
- const apr_uint32_t factor = 0xd1f3da69;
-#endif
-
/* Calculate the hash value
(HASH_VALUE has been initialized to REVISION).
@@ -233,35 +228,8 @@ hash_func(svn_revnum_t revision,
make as much of *PATH influence the result as possible to get an "even"
spread across the hash buckets (maximizes our cache retention rate and
thus the hit rates).
-
- When chunked access is possible (independent of the PATH pointer's
- value!), we read 4 bytes at once and multiply the hash value with a
- FACTOR that mirror / pattern / shift all 4 input bytes to various bits
- of the result. The final result will be taken from the MSBs.
-
- When chunked access is not possible (not supported by CPU or odd bytes
- at the end of *PATH), we use the simple traditional "* 33" hash
- function that works very well with texts / paths and that e.g. APR uses.
-
- Please note that the bytewise and the chunked calculation are *NOT*
- interchangeable as they will yield different results for the same input.
- For any given machine and *PATH, we must use a fixed combination of the
- two functions.
*/
- i = 0;
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* We relax the dependency chain between iterations by processing
- two chunks from the input per hash_value self-multiplication.
- The HASH_VALUE update latency is now 1 MUL latency + 1 ADD latency
- per 2 chunks instead of 1 chunk.
- */
- for (; i + 8 <= path_len; i += 8)
- hash_value = hash_value * factor * factor
- + ( *(const apr_uint32_t*)(path + i) * factor
- + *(const apr_uint32_t*)(path + i + 4));
-#endif
-
- for (; i < path_len; ++i)
+ for (i = 0; i < path_len; ++i)
/* Help GCC to minimize the HASH_VALUE update latency by splitting the
MUL 33 of the naive implementation: h = h * 33 + path[i]. This
shortens the dependency chain from 1 shift + 2 ADDs to 1 shift + 1 ADD.
Index: subversion/libsvn_fs_x/dag_cache.c
===================================================================
--- subversion/libsvn_fs_x/dag_cache.c (revision 1924098)
+++ subversion/libsvn_fs_x/dag_cache.c (working copy)
@@ -312,11 +312,6 @@ cache_lookup(svn_fs_x__dag_cache_t *cache,
apr_size_t path_len = path->len;
apr_uint32_t hash_value = (apr_uint32_t)(apr_uint64_t)change_set;
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* "randomizing" / distributing factor used in our hash function */
- const apr_uint32_t factor = 0xd1f3da69;
-#endif
-
/* optimistic lookup: hit the same bucket again? */
cache_entry_t *result = &cache->buckets[cache->last_hit];
if ( (result->change_set == change_set)
@@ -332,20 +327,7 @@ cache_lookup(svn_fs_x__dag_cache_t *cache,
/* need to do a full lookup. Calculate the hash value
(HASH_VALUE has been initialized to REVISION). */
- i = 0;
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* We relax the dependency chain between iterations by processing
- two chunks from the input per hash_value self-multiplication.
- The HASH_VALUE update latency is now 1 MUL latency + 1 ADD latency
- per 2 chunks instead of 1 chunk.
- */
- for (; i + 8 <= path_len; i += 8)
- hash_value = hash_value * factor * factor
- + ( *(const apr_uint32_t*)(path->data + i) * factor
- + *(const apr_uint32_t*)(path->data + i + 4));
-#endif
-
- for (; i < path_len; ++i)
+ for (i = 0; i < path_len; ++i)
/* Help GCC to minimize the HASH_VALUE update latency by splitting the
MUL 33 of the naive implementation: h = h * 33 + path[i]. This
shortens the dependency chain from 1 shift + 2 ADDs to 1 shift + 1 ADD.
Index: subversion/libsvn_fs_x/string_table.c
===================================================================
--- subversion/libsvn_fs_x/string_table.c (revision 1924098)
+++ subversion/libsvn_fs_x/string_table.c (working copy)
@@ -473,21 +473,6 @@ svn_fs_x__string_table_create(const string_table_b
return result;
}
-/* Masks used by table_copy_string. copy_mask[I] is used if the target
- content to be preserved starts at byte I within the current chunk.
- This is used to work around alignment issues.
- */
-#if SVN_UNALIGNED_ACCESS_IS_OK
-static const char *copy_masks[8] = { "\xff\xff\xff\xff\xff\xff\xff\xff",
- "\x00\xff\xff\xff\xff\xff\xff\xff",
- "\x00\x00\xff\xff\xff\xff\xff\xff",
- "\x00\x00\x00\xff\xff\xff\xff\xff",
- "\x00\x00\x00\x00\xff\xff\xff\xff",
- "\x00\x00\x00\x00\x00\xff\xff\xff",
- "\x00\x00\x00\x00\x00\x00\xff\xff",
- "\x00\x00\x00\x00\x00\x00\x00\xff" };
-#endif
-
static void
table_copy_string(char *buffer,
apr_size_t len,
@@ -499,40 +484,10 @@ table_copy_string(char *buffer,
{
assert(header->head_length <= len);
{
-#if SVN_UNALIGNED_ACCESS_IS_OK
- /* the sections that we copy tend to be short but we can copy
- *all* of it chunky because we made sure that source and target
- buffer have some extra padding to prevent segfaults. */
- apr_uint64_t mask;
- apr_size_t to_copy = len - header->head_length;
- apr_size_t copied = 0;
-
- const char *source = table->data + header->tail_start;
- char *target = buffer + header->head_length;
- len = header->head_length;
-
- /* copy whole chunks */
- while (to_copy >= copied + sizeof(apr_uint64_t))
- {
- *(apr_uint64_t *)(target + copied)
- = *(const apr_uint64_t *)(source + copied);
- copied += sizeof(apr_uint64_t);
- }
-
- /* copy the remainder assuming that we have up to 8 extra bytes
- of addressable buffer on the source and target sides.
- Now, we simply copy 8 bytes and use a mask to filter & merge
- old with new data. */
- mask = *(const apr_uint64_t *)copy_masks[to_copy - copied];
- *(apr_uint64_t *)(target + copied)
- = (*(apr_uint64_t *)(target + copied) & mask)
- | (*(const apr_uint64_t *)(source + copied) & ~mask);
-#else
memcpy(buffer + header->head_length,
table->data + header->tail_start,
len - header->head_length);
len = header->head_length;
-#endif
}
header = &table->short_strings[header->head_string];
Index: subversion/libsvn_subr/eol.c
===================================================================
--- subversion/libsvn_subr/eol.c (revision 1924098)
+++ subversion/libsvn_subr/eol.c (working copy)
@@ -33,34 +33,6 @@
char *
svn_eol__find_eol_start(char *buf, apr_size_t len)
{
-#if SVN_UNALIGNED_ACCESS_IS_OK
-
- /* Scan the input one machine word at a time. */
- for (; len > sizeof(apr_uintptr_t)
- ; buf += sizeof(apr_uintptr_t), len -= sizeof(apr_uintptr_t))
- {
- /* This is a variant of the well-known strlen test: */
- apr_uintptr_t chunk = *(const apr_uintptr_t *)buf;
-
- /* A byte in SVN__R_TEST is \0, iff it was \r in *BUF.
- * Similarly, SVN__N_TEST is an indicator for \n. */
- apr_uintptr_t r_test = chunk ^ SVN__R_MASK;
- apr_uintptr_t n_test = chunk ^ SVN__N_MASK;
-
- /* A byte in SVN__R_TEST can only be < 0x80, iff it has been \0 before
- * (i.e. \r in *BUF). Ditto for SVN__N_TEST. */
- r_test |= (r_test & SVN__LOWER_7BITS_SET) + SVN__LOWER_7BITS_SET;
- n_test |= (n_test & SVN__LOWER_7BITS_SET) + SVN__LOWER_7BITS_SET;
-
- /* Check whether at least one of the words contains a byte <0x80
- * (if one is detected, there was a \r or \n in CHUNK). */
- if ((r_test & n_test & SVN__BIT_7_SET) != SVN__BIT_7_SET)
- break;
- }
-
-#endif
-
- /* The remaining odd bytes will be examined the naive way: */
for (; len > 0; ++buf, --len)
{
if (*buf == '\n' || *buf == '\r')
Index: subversion/libsvn_subr/hash.c
===================================================================
--- subversion/libsvn_subr/hash.c (revision 1924098)
+++ subversion/libsvn_subr/hash.c (working copy)
@@ -636,18 +636,8 @@ hashfunc_compatible(const char *char_key, apr_ssiz
if (*klen == APR_HASH_KEY_STRING)
*klen = strlen(char_key);
-#if SVN_UNALIGNED_ACCESS_IS_OK
for (p = key, i = *klen; i >= 4; i-=4, p+=4)
{
- apr_uint32_t chunk = *(const apr_uint32_t *)p;
-
- /* the ">> 17" part gives upper bits in the chunk a chance to make
- some impact as well */
- hash = hash * 33 * 33 * 33 * 33 + chunk + (chunk >> 17);
- }
-#else
- for (p = key, i = *klen; i >= 4; i-=4, p+=4)
- {
hash = hash * 33 * 33 * 33 * 33
+ p[0] * 33 * 33 * 33
+ p[1] * 33 * 33
@@ -654,7 +644,7 @@ hashfunc_compatible(const char *char_key, apr_ssiz
+ p[2] * 33
+ p[3];
}
-#endif
+
for (; i; i--, p++)
hash = hash * 33 + *p;
Index: subversion/libsvn_subr/string.c
===================================================================
--- subversion/libsvn_subr/string.c (revision 1924098)
+++ subversion/libsvn_subr/string.c (working copy)
@@ -1508,20 +1508,6 @@ svn_cstring__match_length(const char *a,
{
apr_size_t pos = 0;
-#if SVN_UNALIGNED_ACCESS_IS_OK
-
- /* Chunky processing is so much faster ...
- *
- * We can't make this work on architectures that require aligned access
- * because A and B will probably have different alignment. So, skipping
- * the first few chars until alignment is reached is not an option.
- */
- for (; max_len - pos >= sizeof(apr_size_t); pos += sizeof(apr_size_t))
- if (*(const apr_size_t*)(a + pos) != *(const apr_size_t*)(b + pos))
- break;
-
-#endif
-
for (; pos < max_len; ++pos)
if (a[pos] != b[pos])
break;
@@ -1536,22 +1522,6 @@ svn_cstring__reverse_match_length(const char *a,
{
apr_size_t pos = 0;
-#if SVN_UNALIGNED_ACCESS_IS_OK
-
- /* Chunky processing is so much faster ...
- *
- * We can't make this work on architectures that require aligned access
- * because A and B will probably have different alignment. So, skipping
- * the first few chars until alignment is reached is not an option.
- */
- for (pos = sizeof(apr_size_t); pos <= max_len; pos += sizeof(apr_size_t))
- if (*(const apr_size_t*)(a - pos) != *(const apr_size_t*)(b - pos))
- break;
-
- pos -= sizeof(apr_size_t);
-
-#endif
-
/* If we find a mismatch at -pos, pos-1 characters matched.
*/
while (++pos <= max_len)
Index: subversion/libsvn_subr/utf_validate.c
===================================================================
--- subversion/libsvn_subr/utf_validate.c (revision 1924098)
+++ subversion/libsvn_subr/utf_validate.c (working copy)
@@ -258,17 +258,6 @@ static const char machine [9][14] = {
static const char *
first_non_fsm_start_char(const char *data, apr_size_t max_len)
{
-#if SVN_UNALIGNED_ACCESS_IS_OK
-
- /* Scan the input one machine word at a time. */
- for (; max_len > sizeof(apr_uintptr_t)
- ; data += sizeof(apr_uintptr_t), max_len -= sizeof(apr_uintptr_t))
- if (*(const apr_uintptr_t *)data & SVN__BIT_7_SET)
- break;
-
-#endif
-
- /* The remaining odd bytes will be examined the naive way: */
for (; max_len > 0; ++data, --max_len)
if ((unsigned char)*data >= 0x80)
break;
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Branko Čibej <br...@apache.org>.
On 28. 2. 25 13:45, Branko Čibej wrote:
> On 26. 2. 25 15:20, Stefan Sperling wrote:
>> On Wed, Feb 26, 2025 at 02:54:43PM +0100, Daniel Sahlberg wrote:
>>> Should have researched a bit more before replying. There was already some
>>> code to do exactly this, but it was removed in r1722879:
>>>
>>> [[[
>>> stefan2 2016-01-04 15:17:04
>>> Stop using pointer arithmetics to check for proper alignment because
>>> that is not portable.
>>>
>>> As a result, platforms that don't allow unaligned data access will
>>> suffer a small additional performance hit.
>>>
>>> * subversion/libsvn_subr/eol.c
>>> (svn_eol__find_eol_start): No longer attempt aligned chunky processing
>>> when unaligned access is not supported.
>>>
>>> * subversion/libsvn_subr/utf_validate.c
>>> (first_non_fsm_start_char): Same.
>>> ]]]
>>>
>>> Don't quite understand why but there is probably a reasonable explanation.
>> It took us a few rounds of testing to keep things working across
>> all platforms when these optimizations went in.
>>
>> As far as I recall, there was run-time fallout on the OpenBSD/sparc64
>> buildbot (which no longer exists; I don't have sparc64 hardware anymore).
>> Unaligned access will always cause hardware traps on that platform.
>> The above change was probably related to that.
>>
>> With x86 platform behaviour changing all the testing we did back then
>> is being invalidated. The solution with the least amount of effort on
>> our part would be using the simple char * loop implementation everywhere.
>> The maintenance effort required to keep the unaligned-access code path
>> working and safe on all (modern) platforms would likely exceed our
>> current development capabilities. Though of course, if someone wants
>> to do that work, I wouldn't be opposed.
>
> Actually, x86 behaviour hasn't changed wrt alignment for "normal"
> memory access in decades. What changed was the addition of vector
> instructions, which explicitly disallow unaligned access, and compiler
> support for them -- in the reported case it's the -mavx that does it,
> while I suspect clang just adds something similar by default at high
> enough optimization levels.
>
> At the time, IIRC stefan² tested performance on some (then-)modern x86
> CPUs and came away with the result that it's faster to allow the CPU
> to handle unaligned access ... or it would be. These kinds of
> optimizations always smelled slightly off to me, e.g., why not allow
> compiler intrinsics to do their magic instead?
>
> For my €.02, I'd just remove this macro and the micro-optimizations
> that go with it and, wherever possible, use standard library functions
> instead of the handcrafted ones.
>
> Or, since the macro is public -- deprecate it and stop using it in our
> code. The workaround for the reported bug would be adding
> -DSVN_UNALIGNED_ACCESS_IS_OK=0 to the CFLAGS.
This is probably the minimal patch to disable this tweak. There's a
bunch of code that needs to be replaced or removed, though.
[[[
Index: notes/knobs
===================================================================
--- notes/knobs (revision 1924097)
+++ notes/knobs (working copy)
@@ -38,7 +38,7 @@ SUFFIX_LINES_TO_KEEP
SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR
SVN_FS_FS_MAX_LINEAR_DELTIFICATION
SVN_FS_FS_MAX_DELTIFICATION_WALK
-SVN_UNALIGNED_ACCESS_IS_OK
+SVN_UNALIGNED_ACCESS_IS_OK (deprecated)
2.2 Features
@@ -164,12 +164,14 @@ SVN_I_LIKE_LATENCY_SO_IGNORE_HTTPV2
3.8 SVN_UNALIGNED_ACCESS_IS_OK
+ DEPRECATED, unused. The rest of this entry is historical.
+
Scope: (global)
Purpose: enable data accesss optimizations.
If your target CPU supports unaligned memory access without
significant performance penalties, you should enable this
optimization as it allows for processing 4 or 8 bytes at
- once instead of just one byte at a time.
+ once instead of just one byte at a time.
Range: 0 1
Default: platform dependant (see svn_types.h)
Suggested: 0
Index: subversion/include/svn_types.h
===================================================================
--- subversion/include/svn_types.h (revision 1924097)
+++ subversion/include/svn_types.h (working copy)
@@ -124,16 +124,12 @@ extern "C" {
* if unaligned access is supported for integers.
*
* @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.14 API.
*/
-#ifndef SVN_UNALIGNED_ACCESS_IS_OK
-# if defined(_M_IX86) || defined(i386) \
- || defined(_M_X64) || defined(__x86_64) \
- || defined(__powerpc__) || defined(__ppc__)
-# define SVN_UNALIGNED_ACCESS_IS_OK 1
-# else
-# define SVN_UNALIGNED_ACCESS_IS_OK 0
-# endif
+#ifdef SVN_UNALIGNED_ACCESS_IS_OK
+# undef SVN_UNALIGNED_ACCESS_IS_OK
#endif
+#define SVN_UNALIGNED_ACCESS_IS_OK 0
]]]
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Branko Čibej <br...@apache.org>.
On 26. 2. 25 15:20, Stefan Sperling wrote:
> On Wed, Feb 26, 2025 at 02:54:43PM +0100, Daniel Sahlberg wrote:
>> Should have researched a bit more before replying. There was already some
>> code to do exactly this, but it was removed in r1722879:
>>
>> [[[
>> stefan2 2016-01-04 15:17:04
>> Stop using pointer arithmetics to check for proper alignment because
>> that is not portable.
>>
>> As a result, platforms that don't allow unaligned data access will
>> suffer a small additional performance hit.
>>
>> * subversion/libsvn_subr/eol.c
>> (svn_eol__find_eol_start): No longer attempt aligned chunky processing
>> when unaligned access is not supported.
>>
>> * subversion/libsvn_subr/utf_validate.c
>> (first_non_fsm_start_char): Same.
>> ]]]
>>
>> Don't quite understand why but there is probably a reasonable explanation.
> It took us a few rounds of testing to keep things working across
> all platforms when these optimizations went in.
>
> As far as I recall, there was run-time fallout on the OpenBSD/sparc64
> buildbot (which no longer exists; I don't have sparc64 hardware anymore).
> Unaligned access will always cause hardware traps on that platform.
> The above change was probably related to that.
>
> With x86 platform behaviour changing all the testing we did back then
> is being invalidated. The solution with the least amount of effort on
> our part would be using the simple char * loop implementation everywhere.
> The maintenance effort required to keep the unaligned-access code path
> working and safe on all (modern) platforms would likely exceed our
> current development capabilities. Though of course, if someone wants
> to do that work, I wouldn't be opposed.
Actually, x86 behaviour hasn't changed wrt alignment for "normal" memory
access in decades. What changed was the addition of vector instructions,
which explicitly disallow unaligned access, and compiler support for
them -- in the reported case it's the -mavx that does it, while I
suspect clang just adds something similar by default at high enough
optimization levels.
At the time, IIRC stefan² tested performance on some (then-)modern x86
CPUs and came away with the result that it's faster to allow the CPU to
handle unaligned access ... or it would be. These kinds of optimizations
always smelled slightly off to me, e.g., why not allow compiler
intrinsics to do their magic instead?
For my €.02, I'd just remove this macro and the micro-optimizations that
go with it and, wherever possible, use standard library functions
instead of the handcrafted ones.
Or, since the macro is public -- deprecate it and stop using it in our
code. The workaround for the reported bug would be adding
-DSVN_UNALIGNED_ACCESS_IS_OK=0 to the CFLAGS.
-- Brane
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Stefan Sperling <st...@apache.org>.
On Wed, Feb 26, 2025 at 02:54:43PM +0100, Daniel Sahlberg wrote:
> Should have researched a bit more before replying. There was already some
> code to do exactly this, but it was removed in r1722879:
>
> [[[
> stefan2 2016-01-04 15:17:04
> Stop using pointer arithmetics to check for proper alignment because
> that is not portable.
>
> As a result, platforms that don't allow unaligned data access will
> suffer a small additional performance hit.
>
> * subversion/libsvn_subr/eol.c
> (svn_eol__find_eol_start): No longer attempt aligned chunky processing
> when unaligned access is not supported.
>
> * subversion/libsvn_subr/utf_validate.c
> (first_non_fsm_start_char): Same.
> ]]]
>
> Don't quite understand why but there is probably a reasonable explanation.
It took us a few rounds of testing to keep things working across
all platforms when these optimizations went in.
As far as I recall, there was run-time fallout on the OpenBSD/sparc64
buildbot (which no longer exists; I don't have sparc64 hardware anymore).
Unaligned access will always cause hardware traps on that platform.
The above change was probably related to that.
With x86 platform behaviour changing all the testing we did back then
is being invalidated. The solution with the least amount of effort on
our part would be using the simple char * loop implementation everywhere.
The maintenance effort required to keep the unaligned-access code path
working and safe on all (modern) platforms would likely exceed our
current development capabilities. Though of course, if someone wants
to do that work, I wouldn't be opposed.
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 26 feb. 2025 kl 14:45 skrev Daniel Sahlberg <
daniel.l.sahlberg@gmail.com>:
> Den ons 26 feb. 2025 kl 14:30 skrev Stefan Sperling <st...@apache.org>:
>
>> On Wed, Feb 26, 2025 at 01:24:22PM +0100, Daniel Sahlberg wrote:
>> > Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was
>> > set to 0 (so the char by char loop after #endif is used instead), the
>> code
>> > would run just fine. Can you confirm that?
>> >
>> > SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to
>> > change that for GCC15.
>>
>> Unaligned access is undefined behaviour (UB). Compilers are now taking
>> more liberties with undefined behaviour than they used to. So we need
>> to be very careful in our assumptions about UB.
>>
>> Our code which is guarded by SVN_UNALIGNED_ACCESS_IS_OK might become
>> unsafe when built with newer compilers on any platform. This code's
>> behaviour is no longer under our control, but the compiler's s control.
>> svn_eol__find_eol_start() might crash, become a no-op, or have unknown
>> side-effects resulting in annoyances or even CVEs.
>>
>> At the very least, we should have this macro off by default and require
>> it to be enabled manually. For best safety, the code should be removed.
>> I understand that removing this code will incur a performance cost.
>> The code was backed up by elaborate performance testing when it was
>> written years ago.
>>
>> But what modern compilers make of UB will likely keep changing for the
>> worse (or better, depending on which side of the UB-fence you are on),
>> violating assumptions about platform behaviour in the macro definition:
>>
>> > #ifndef SVN_UNALIGNED_ACCESS_IS_OK
>> > # if defined(_M_IX86) || defined(i386) \
>> > || defined(_M_X64) || defined(__x86_64) \
>> > || defined(__powerpc__) || defined(__ppc__)
>> > # define SVN_UNALIGNED_ACCESS_IS_OK 1
>> > # else
>> > # define SVN_UNALIGNED_ACCESS_IS_OK 0
>> > # endif
>> > #endif
>>
>> I have seen contrived programs being translated into nothing on x86 when
>> built with a relatively recent version of clang (16) at optimization
>> levels
>> of -O2 and up. For example, the following program will literally have its
>> entire main() function elided:
>>
>> [[[
>> #include <float.h>
>> int main(void) {
>> double v = DBL_MAX;
>> int v2 = v;
>> if (v2 != v) return 0;
>> return 1;
>> }
>> ]]]
>> (courtesy of https://bsd.network/@kristapsdz/113870782376321887)
>>
>> On OpenBSD/amd64, with clang -O2 this program compiles to 3 instructions,
>> an empty function. The resulting executable just crashes when run.
>> [[[
>> (gdb) disassemble main
>> 0x0000000000001950 <+0>: endbr64
>> 0x0000000000001954 <+4>: push %rbp
>> 0x0000000000001955 <+5>: mov %rsp,%rbp
>> ]]]
>>
>> At -01 there are 4 lines (still crashes). Without optimization (-O0)
>> there are 38 lines of assembly instead of 3, and the program exits
>> with status 0.
>>
>> Granted, what I have seen are contrived cases. But they resulted
>> from someone looking into problems while doing development in C.
>> As a C developer, this scares me enough to avoid UB at all costs
>> because I cannot trust the compiler to warn about my mistakes
>> which introduce UB. Rather, the compiler might decide to elide
>> the code and change the intended meaning of the program.
>>
>
> Thanks for the detailed explanation. It seems really ugly.
>
> Would add a separate for-loop before "one machine word at a time",
> iterating until buf is aligned (maximum sizeof(void*)-1 times) help here?
>
Should have researched a bit more before replying. There was already some
code to do exactly this, but it was removed in r1722879:
[[[
stefan2 2016-01-04 15:17:04
Stop using pointer arithmetics to check for proper alignment because
that is not portable.
As a result, platforms that don't allow unaligned data access will
suffer a small additional performance hit.
* subversion/libsvn_subr/eol.c
(svn_eol__find_eol_start): No longer attempt aligned chunky processing
when unaligned access is not supported.
* subversion/libsvn_subr/utf_validate.c
(first_non_fsm_start_char): Same.
]]]
Don't quite understand why but there is probably a reasonable explanation.
>
> Any idea of the performance gain on SVN_UNALIGNED_ACCESS_IS_OK on modern
> hardware/modern compilers?
>
> Cheers,
> Daniel
>
>
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 26 feb. 2025 kl 14:30 skrev Stefan Sperling <st...@apache.org>:
> On Wed, Feb 26, 2025 at 01:24:22PM +0100, Daniel Sahlberg wrote:
> > Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was
> > set to 0 (so the char by char loop after #endif is used instead), the
> code
> > would run just fine. Can you confirm that?
> >
> > SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to
> > change that for GCC15.
>
> Unaligned access is undefined behaviour (UB). Compilers are now taking
> more liberties with undefined behaviour than they used to. So we need
> to be very careful in our assumptions about UB.
>
> Our code which is guarded by SVN_UNALIGNED_ACCESS_IS_OK might become
> unsafe when built with newer compilers on any platform. This code's
> behaviour is no longer under our control, but the compiler's s control.
> svn_eol__find_eol_start() might crash, become a no-op, or have unknown
> side-effects resulting in annoyances or even CVEs.
>
> At the very least, we should have this macro off by default and require
> it to be enabled manually. For best safety, the code should be removed.
> I understand that removing this code will incur a performance cost.
> The code was backed up by elaborate performance testing when it was
> written years ago.
>
> But what modern compilers make of UB will likely keep changing for the
> worse (or better, depending on which side of the UB-fence you are on),
> violating assumptions about platform behaviour in the macro definition:
>
> > #ifndef SVN_UNALIGNED_ACCESS_IS_OK
> > # if defined(_M_IX86) || defined(i386) \
> > || defined(_M_X64) || defined(__x86_64) \
> > || defined(__powerpc__) || defined(__ppc__)
> > # define SVN_UNALIGNED_ACCESS_IS_OK 1
> > # else
> > # define SVN_UNALIGNED_ACCESS_IS_OK 0
> > # endif
> > #endif
>
> I have seen contrived programs being translated into nothing on x86 when
> built with a relatively recent version of clang (16) at optimization levels
> of -O2 and up. For example, the following program will literally have its
> entire main() function elided:
>
> [[[
> #include <float.h>
> int main(void) {
> double v = DBL_MAX;
> int v2 = v;
> if (v2 != v) return 0;
> return 1;
> }
> ]]]
> (courtesy of https://bsd.network/@kristapsdz/113870782376321887)
>
> On OpenBSD/amd64, with clang -O2 this program compiles to 3 instructions,
> an empty function. The resulting executable just crashes when run.
> [[[
> (gdb) disassemble main
> 0x0000000000001950 <+0>: endbr64
> 0x0000000000001954 <+4>: push %rbp
> 0x0000000000001955 <+5>: mov %rsp,%rbp
> ]]]
>
> At -01 there are 4 lines (still crashes). Without optimization (-O0)
> there are 38 lines of assembly instead of 3, and the program exits
> with status 0.
>
> Granted, what I have seen are contrived cases. But they resulted
> from someone looking into problems while doing development in C.
> As a C developer, this scares me enough to avoid UB at all costs
> because I cannot trust the compiler to warn about my mistakes
> which introduce UB. Rather, the compiler might decide to elide
> the code and change the intended meaning of the program.
>
Thanks for the detailed explanation. It seems really ugly.
Would add a separate for-loop before "one machine word at a time",
iterating until buf is aligned (maximum sizeof(void*)-1 times) help here?
Any idea of the performance gain on SVN_UNALIGNED_ACCESS_IS_OK on modern
hardware/modern compilers?
Cheers,
Daniel
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Stefan Sperling <st...@apache.org>.
On Wed, Feb 26, 2025 at 01:24:22PM +0100, Daniel Sahlberg wrote:
> Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was
> set to 0 (so the char by char loop after #endif is used instead), the code
> would run just fine. Can you confirm that?
>
> SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to
> change that for GCC15.
Unaligned access is undefined behaviour (UB). Compilers are now taking
more liberties with undefined behaviour than they used to. So we need
to be very careful in our assumptions about UB.
Our code which is guarded by SVN_UNALIGNED_ACCESS_IS_OK might become
unsafe when built with newer compilers on any platform. This code's
behaviour is no longer under our control, but the compiler's s control.
svn_eol__find_eol_start() might crash, become a no-op, or have unknown
side-effects resulting in annoyances or even CVEs.
At the very least, we should have this macro off by default and require
it to be enabled manually. For best safety, the code should be removed.
I understand that removing this code will incur a performance cost.
The code was backed up by elaborate performance testing when it was
written years ago.
But what modern compilers make of UB will likely keep changing for the
worse (or better, depending on which side of the UB-fence you are on),
violating assumptions about platform behaviour in the macro definition:
> #ifndef SVN_UNALIGNED_ACCESS_IS_OK
> # if defined(_M_IX86) || defined(i386) \
> || defined(_M_X64) || defined(__x86_64) \
> || defined(__powerpc__) || defined(__ppc__)
> # define SVN_UNALIGNED_ACCESS_IS_OK 1
> # else
> # define SVN_UNALIGNED_ACCESS_IS_OK 0
> # endif
> #endif
I have seen contrived programs being translated into nothing on x86 when
built with a relatively recent version of clang (16) at optimization levels
of -O2 and up. For example, the following program will literally have its
entire main() function elided:
[[[
#include <float.h>
int main(void) {
double v = DBL_MAX;
int v2 = v;
if (v2 != v) return 0;
return 1;
}
]]]
(courtesy of https://bsd.network/@kristapsdz/113870782376321887)
On OpenBSD/amd64, with clang -O2 this program compiles to 3 instructions,
an empty function. The resulting executable just crashes when run.
[[[
(gdb) disassemble main
0x0000000000001950 <+0>: endbr64
0x0000000000001954 <+4>: push %rbp
0x0000000000001955 <+5>: mov %rsp,%rbp
]]]
At -01 there are 4 lines (still crashes). Without optimization (-O0)
there are 38 lines of assembly instead of 3, and the program exits
with status 0.
Granted, what I have seen are contrived cases. But they resulted
from someone looking into problems while doing development in C.
As a C developer, this scares me enough to avoid UB at all costs
because I cannot trust the compiler to warn about my mistakes
which introduce UB. Rather, the compiler might decide to elide
the code and change the intended meaning of the program.
Re: Unaligned access in svn_eol__find_eol_start causes crash w/ upcoming GCC 15
Posted by Daniel Sahlberg <da...@gmail.com>.
Den ons 26 feb. 2025 kl 12:33 skrev Sam James <sa...@gentoo.org>:
> Hi!
>
Hi,
Thanks for the detailed report!
> When subversion-1.14.5 is built with upcoming GCC 15 with -O3 -mavx (or
> -O2 -mavx -fno-vect-cost-model), it crashes in some cases in
> svn_eol__find_eol_start.
>
> We found `svn checkout https://svn.code.sf.net/p/sdcc/code/trunk/sdcc`
> <https://svn.code.sf.net/p/sdcc/code/trunk/sdcc>
> was a reliable reproducer, as was `svn cleanup sdcc` once the first
> command crashed. (The svn testsuite passes, though.)
>
> Full backtrace:
> ```
> Thread 1 (Thread 0x7fb5f533d7c0 (LWP 418455)):
> #0 0x00007fb5f5be084d in svn_eol__find_eol_start (buf=<optimized out>,
> buf@entry=0x564804d4a48b " bug3379723.c\n*/\n\n#include
> <testfwk.h>\n\n#if !defined(PORT_HOST) && !defined(__SDCC_hc08) &&
> !defined(__SDCC_s08) && !defined(__SDCC_mos6502) &&
> !defined(__SDCC_mos65c02) && !defined(__SDCC_sm83) && "..., len=<optimized
> out>, len@entry=838) at subversion/libsvn_subr/eol.c:43
> #1 0x00007fb5f5c0d2ca in translate_chunk (dst=dst@entry=0x564804d56840,
> b=0x56480490dec0, buf=<optimized out>, buflen=<optimized out>,
> pool=<optimized out>) at subversion/libsvn_subr/subst.c:1152
> #2 0x00007fb5f5c0e91d in translated_stream_read (baton=0x56480490de20,
> buffer=0x564804d4f498 "/*\n bug2686159.c\n */\n\n#include
> <testfwk.h>\n\n#ifdef __SDCC_pic16\n# define ADDRESS(x) (0x02 ## x)\n#elif
> defined(__SDCC_pic14)\n# define ADDRESS(x) (0x01A0 + 0x ## x)\n#elif
> defined(__SDCC_stm8)\n# define"..., len=0x7ffe9abfae10) at
> subversion/libsvn_subr/subst.c:1297
> #3 0x00007fb5f5c08647 in svn_stream_copy3 (from=0x56480490de60,
> to=0x564804d49548, cancel_func=cancel_func@entry=0x7fb5f5bcffb0
> <check_cancel>, cancel_baton=cancel_baton@entry=0x0,
> scratch_pool=scratch_pool@entry=0x56480490c6f8) at
> subversion/libsvn_subr/stream.c:423
> #4 0x00007fb5f5d01d45 in run_file_install (wqb=0x7ffe9abfaf90,
> db=0x56480464e268, work_item=<optimized out>, wri_abspath=0x5648049de410
> "/home/kostadin/sdcc/support/regression/tests", cancel_func=0x7fb5f5bcffb0
> <check_cancel>, cancel_baton=0x0, scratch_pool=0x56480490c6f8) at
> subversion/libsvn_wc/workqueue.c:580
> #5 0x00007fb5f5d02fdd in dispatch_work_item (wqb=<optimized out>,
> db=<optimized out>, wri_abspath=<optimized out>, work_item=<optimized out>,
> cancel_func=<optimized out>, cancel_baton=<optimized out>,
> scratch_pool=<optimized out>) at subversion/libsvn_wc/workqueue.c:1441
> #6 svn_wc__wq_run (db=0x56480464e268, wri_abspath=0x5648049de410
> "/home/kostadin/sdcc/support/regression/tests", cancel_func=0x7fb5f5bcffb0
> <check_cancel>, cancel_baton=0x0, scratch_pool=scratch_pool@entry=0x5648049de2a8)
> at subversion/libsvn_wc/workqueue.c:1546
> #7 0x00007fb5f5cc4f0c in close_directory (dir_baton=0x5648049de320,
> pool=<optimized out>) at subversion/libsvn_wc/update_editor.c:2819
> #8 0x00007fb5f55c89b3 in maybe_close_dir (dir=<optimized out>) at
> subversion/libsvn_ra_serf/update.c:738
> #9 0x00007fb5f55ce0fc in handle_response_cb (request=0x5648048aee18,
> response=<optimized out>, baton=0x5648048a0ff0,
> response_pool=0x564804d46468) at subversion/libsvn_ra_serf/util.c:1540
> #10 0x00007fb5f5393f8e in serf.process_connection () from
> /usr/lib64/libserf-1.so.1
> #11 0x00007fb5f5394815 in serf_event_trigger () from
> /usr/lib64/libserf-1.so.1
> #12 0x00007fb5f5394989 in serf_context_run () from
> /usr/lib64/libserf-1.so.1
> #13 0x00007fb5f55cd7b1 in svn_ra_serf__context_run (sess=0x564804679c60,
> waittime_left=0x7ffe9abfb3a0, scratch_pool=0x5648047128b8) at
> subversion/libsvn_ra_serf/util.c:913
> #14 svn_ra_serf__context_run (sess=0x564804679c60,
> waittime_left=0x7ffe9abfb3a0, scratch_pool=0x5648047128b8) at
> subversion/libsvn_ra_serf/util.c:902
> #15 0x00007fb5f55c998d in process_editor_report (ctx=0x564804661b28,
> handler=0x564804690a08, scratch_pool=0x564804690878) at
> subversion/libsvn_ra_serf/update.c:2429
> #16 finish_report (report_baton=0x564804661b28, pool=<optimized out>) at
> subversion/libsvn_ra_serf/update.c:2504
> #17 0x00007fb5f5c868d0 in svn_wc_crawl_revisions5 (wc_ctx=<optimized out>,
> local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc",
> reporter=0x7fb5f55df960 <ra_serf_reporter>, report_baton=0x564804661b28,
> restore_files=restore_files@entry=1, depth=depth@entry=svn_depth_unknown,
> honor_depth_exclude=1, depth_compatibility_trick=0, use_commit_times=0,
> cancel_func=0x7fb5f5bcffb0 <check_cancel>, cancel_baton=0x0,
> notify_func=0x5647fc07f070 <svn_cl__check_externals_failed_notify_wrapper>,
> notify_baton=0x7ffe9abfb9e0, scratch_pool=0x56480465f788) at
> subversion/libsvn_wc/adm_crawler.c:859
> #18 0x00007fb5f5dbb741 in update_internal (result_rev=result_rev@entry=0x0,
> timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c,
> conflicted_paths=conflicted_paths@entry=0x0, ra_session_p=<optimized
> out>, local_abspath=local_abspath@entry=0x56480465f8a0
> "/home/kostadin/sdcc", anchor_abspath=anchor_abspath@entry=0x564804661350
> "/home/kostadin/sdcc", revision=0x7ffe9abfb790, depth=svn_depth_unknown,
> depth_is_sticky=0, ignore_externals=0, allow_unver_obstructions=0,
> adds_as_modification=1, notify_summary=1, ctx=0x56480464e180,
> result_pool=0x56480465f788, scratch_pool=0x56480465f788) at
> subversion/libsvn_client/update.c:560
> #19 0x00007fb5f5dbbc8c in svn_client__update_internal
> (result_rev=result_rev@entry=0x0, timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c,
> local_abspath=local_abspath@entry=0x56480465f8a0 "/home/kostadin/sdcc",
> revision=revision@entry=0x7ffe9abfb870, depth=depth@entry=svn_depth_unknown,
> depth_is_sticky=depth_is_sticky@entry=1, ignore_externals=0,
> allow_unver_obstructions=0, adds_as_modification=1, make_parents=0,
> innerupdate=0, ra_session=<optimized out>, ctx=0x56480464e180,
> pool=0x56480465f788) at subversion/libsvn_client/update.c:696
> #20 0x00007fb5f5d5336d in svn_client__checkout_internal
> (result_rev=result_rev@entry=0x0, timestamp_sleep=timestamp_sleep@entry=0x7ffe9abfb91c,
> url=url@entry=0x56480465e1f8 "
> https://svn.code.sf.net/p/sdcc/code/trunk/sdcc",
> local_abspath=0x56480465f8a0 "/home/kostadin/sdcc",
> peg_revision=peg_revision@entry=0x7ffe9abfb9d0, revision=revision@entry=0x7ffe9abfb9c0,
> depth=svn_depth_unknown, ignore_externals=0, allow_unver_obstructions=0,
> ra_session=<optimized out>, ctx=0x56480464e180,
> scratch_pool=0x56480465f788) at subversion/libsvn_client/checkout.c:189
> #21 0x00007fb5f5d53594 in svn_client_checkout3 (result_rev=result_rev@entry=0x0,
> URL=0x56480465e1f8 "https://svn.code.sf.net/p/sdcc/code/trunk/sdcc",
> path=<optimized out>, peg_revision=peg_revision@entry=0x7ffe9abfb9d0,
> revision=revision@entry=0x7ffe9abfb9c0, depth=svn_depth_unknown,
> ignore_externals=0, allow_unver_obstructions=0, ctx=0x56480464e180,
> pool=0x56480465f788) at subversion/libsvn_client/checkout.c:218
> #22 0x00005647fc06bef2 in svn_cl__checkout (os=<optimized out>,
> baton=<optimized out>, pool=<optimized out>) at
> subversion/svn/checkout-cmd.c:168
> #23 0x00005647fc08b7eb in sub_main (exit_code=exit_code@entry=0x7ffe9abfbe34,
> argc=argc@entry=3, cmdline_argv=cmdline_argv@entry=0x7ffe9abfbf68,
> pool=pool@entry=0x56480464c638) at subversion/svn/svn.c:3263
> #24 0x00005647fc06983f in main (argc=3, argv=0x7ffe9abfbf68) at
> subversion/svn/svn.c:3348
> ```
>
> The unaligned access itself controlled by SVN_UNALIGNED_ACCESS_IS_OK in
> svn_eol__find_eol_start AFAIK isn't wrong, just the forming and use of
> an unaligned pointer for (apr_)uintptr_t.
>
Looking at the code, I'm assuming that if SVN_UNALIGNED_ACCESS_IS_OK was
set to 0 (so the char by char loop after #endif is used instead), the code
would run just fine. Can you confirm that?
SVN_UNALIGNED_ACCESS_IS_OK is defined as follows - we'd probably have to
change that for GCC15.
#ifndef SVN_UNALIGNED_ACCESS_IS_OK
# if defined(_M_IX86) || defined(i386) \
|| defined(_M_X64) || defined(__x86_64) \
|| defined(__powerpc__) || defined(__ppc__)
# define SVN_UNALIGNED_ACCESS_IS_OK 1
# else
# define SVN_UNALIGNED_ACCESS_IS_OK 0
# endif
#endif
> -fsanitize=undefined (or -fsanitize=alignment, take your pick; it's a
> subset) reports:
> ```
> subversion/libsvn_subr/eol.c:43:21: runtime error: load of misaligned
> address 0x56302447f98b for type 'const apr_uintptr_t', which requires 8
> byte alignment
> 0x56302447f98b: note: pointer points here
> 00 2f 2a 0a 20 20 20 62 75 67 33 33 37 39 37 32 33 2e 63 0a 2a 2f 0a
> 0a 23 69 6e 63 6c 75 64 65
> ^
> ```
>
Cheers,
Daniel