On 26. 5. 26 17:47, Ivan Zhakov wrote:
On Thu, 21 May 2026 at 15:33, Jun Omae <[email protected]> wrote:
On 2026/05/21 21:32, Evgeny Kotkov wrote:
> Jun Omae <[email protected]> writes:
>
>> It occurs only with MSVC toolset v143 (--vsnet-version=2022)
and Whole
>> Program Optimization enabled. It doesn't with v142
(--vsnet-version=2019).
>>
>> svn_fs_fs__delete_revprops_shard() is built by the optimization to
>> delete the "revprops/0" directory even if `shard` argument is 0.
>>
>> Workaround is to disable the optimization.
>
> Interesting. Could be a compiler issue or an UB somewhere in
the code.
>
> Would you mind providing more details on your environment to
make it easier
> to reproduce?
It is able to reproduce using cmake with toolset v143.
1. Pass -vcvars_ver=14.44 to vcvars64.bat
2. Pass CMAKE_INTERPROCEDURAL_OPTIMIZATION=ON to cmake configure
(The option makes Whole Program Optimization enabled in cmake
build)
I can reproduce this issue in my environment and I believe it is
indeed a compiler bug. Here is the behavior across different MSVC
versions:
- MSVC 19.*40*.33821.0: No problem
- MSVC 19.*44*.35227.0: Problem occurs
- MSVC 19.*50*.35730.0: No problem
In version 19.44.35227.0 with /GL (Whole Program Optimization)
enabled, the `if (shard == 0)` branch seems to be incorrrectly
**optimized away**.
I think this is caused by combination of the do-while loop and
the `while (kind == svn_node_dir && to_cleanup > 0)` condition on
the calling side (details below).
The code below from subversion/libsvn_fs_fs/pack.c:synced_pack_shard()
becomes a simple call to svn_io_remove_dir2() that recursively
deletes the revprops/0 directory:
[[[
do
{
SVN_ERR(svn_fs_fs__delete_revprops_shard(revprops_shard_path,
to_cleanup,
ffd->max_files_per_dir,
pb->cancel_func,
pb->cancel_baton,
pool));
/* If the previous shard exists, clean it up as well.
Don't try to clean up shard 0 as it we can't tell quickly
whether it actually needs cleaning up. */
revprops_shard_path = svn_dirent_join(pb->revsprops_dir,
apr_psprintf(pool,"%" APR_INT64_T_FMT, --to_cleanup),
pool);
SVN_ERR(svn_io_check_path(revprops_shard_path, &kind,pool));
}
while (kind ==svn_node_dir && to_cleanup > 0);
]]]
[[[
if (shard == 0)
{
apr_pool_t *iterpool =svn_pool_create(scratch_pool);
int i;
/* delete all files except the one for revision 0 */
for (i = 1; i <max_files_per_dir; ++i)
{
const char *path;
svn_pool_clear(iterpool);
path = svn_dirent_join(shard_path,
apr_psprintf(iterpool,"%d", i),
iterpool);
if (cancel_func)
SVN_ERR(cancel_func(cancel_baton));
SVN_ERR(svn_io_remove_file2(path,TRUE, iterpool));
}
svn_pool_destroy(iterpool);
}
else
SVN_ERR(svn_io_remove_dir2(shard_path,TRUE,
cancel_func,cancel_baton,scratch_pool));
]]]
I checked that replacing the do-while loop with an equivalent while {}
loop makes the problem go away. My theory is that the compiler
incorrectly assumes to_cleanup is > 0 within the loop block and
optimizes away the inner check.
Could there be an even simpler "fix"? Move the --to_cleanup before the
call to svn_dirent_join?
That shouldn't change the semantics at all, but it would indicate that
the MSVC data flow analysis somehow misses that the decrement in the
function parameter has externally visible side effects. The code is
clearly correct.
-- Brane
I have attached several generated assembly files for comparison:
- svn-disasm-vc19.44-orig.asm - code generated by MSVC 19.44 compiler.
- svn-disasm-vc19.44-patched.asm - code where `do {} while` loop
replaced with `while() {}` equivalent generated by MSVC 19.44 compiler
- svn-disasm-vc19.50-orig.asm - code generated by MSVC 19.50 compiler.
--
Ivan Zhakov
