[PATCH] File name too long causing failure to open temporary head file in dlltool

[Martin Storsjö]

On Tue, 13 Aug 2024, jiaying.song.cn@windriver.com wrote:

> From: Jiaying Song <jiaying.song.cn@windriver.com>
>
> During the execution of the command: i686-w64-mingw32-dlltool
> --input-def $def_filepath --output-delaylib $filepath --dllname qemu.exe
> An error occurred:
> i686-w64-mingw32-dlltool: failed to open temporary head file: ..._w64_mingw32_nativesdk_qemu_8_2_2_build_plugins_libqemu_plugin_api_a_h.s
>
> Due to the path length exceeding the Linux system's file name length
> limit (NAME_MAX=255), the temporary file name generated by the
> i686-w64-mingw32-dlltool command becomes too long to open. To address
> this, a new temporary file name prefix is generated using tmp_prefix =
> prefix_encode ("d", getpid()), ensuring that the file name does not
> exceed the system's length limit.
>
> Signed-off-by: Jiaying Song <jiaying.song.cn@windriver.com>
> ---
> binutils/dlltool.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/binutils/dlltool.c b/binutils/dlltool.c
> index 066c99a4..acaeb5a7 100644
> --- a/binutils/dlltool.c
> +++ b/binutils/dlltool.c
> @@ -4099,11 +4099,18 @@ main (int ac, char **av)
>       if (imp_name || delayimp_name)
>         {
>           const char *input = imp_name ? imp_name : delayimp_name;
> -          tmp_prefix = xmalloc (strlen (input) + 2);
> -          sprintf (tmp_prefix, "%s_", input);
> -          for (i = 0; tmp_prefix[i]; i++)
> -            if (!ISALNUM (tmp_prefix[i]))
> -              tmp_prefix[i] = '_';
> +          if ((strlen(input) + 2) > NAME_MAX)
> +            {
> +              tmp_prefix = prefix_encode ("d", getpid ());
> +            }
> +          else
> +            {
> +              tmp_prefix = xmalloc (strlen (input) + 2);
> +              sprintf (tmp_prefix, "%s_", input);
> +              for (i = 0; tmp_prefix[i]; i++)
> +              if (!ISALNUM (tmp_prefix[i]))
> +                tmp_prefix[i] = '_';
> +            }

Thanks for keeping the existing codepath when possible.

Using getpid() here as fallback isn't great, we used to use getpid() 
before c4a8df19ba0a82aa8dea88d9f72ed9e63cb1fa84, but moved away from it. 
(The commit message of that commit is unnecessarily terse - see 
https://sourceware.org/pipermail/binutils/2022-January/119111.html for the 
original commit message.)

Using it as fallback when the normal case is too long shouldn't affect 
those who use it currently with manageable file names - but I wonder if we 
could come up with an even better solution?

To expand on the context here - there are a number of constraints on the 
temp prefix used here. In particular - the temp prefix that is used ends 
up in archive member names within the output library.

- For multiple dlltool invocations running in the same directory in 
parallel, the temp prefix needs to be unique, to avoid temp files 
clashing. Using getpid() does achieve this.

- It would be desireable if the temp prefix is deterministic.

- For two separate libraries with different names, the temp prefix also 
should be unique. (Import libraries are frequently merged into larger 
umbrella libraries. In this case, if two of the input libraries use the 
same temp prefix, the merged library doesn't work as it should.)


It is tempting to only use the file name of the output, rather than the 
full (relative) path - but that causes clashes if we have two dlltool 
processes in parallel, in the same directory, building 
"lib32/libkernel32.a" and "lib64/libkernel32.a". (This exact case was 
fixed in d65c0ddddd85645cab6f11fd711d21638a74489f, see 
https://sourceware.org/pipermail/binutils/2022-March/119999.html and 
https://sourceware.org/bugzilla/show_bug.cgi?id=28885 for references 
around that.)


For this particular case, I wonder if it'd be best to take the last 
(NAME_MAX - 20) chars of the output name, rather than the full string? 
That would retain determinism, and it should hopefully keep the most 
unique parts of the identifier?

Other than that, I don't have a strong objection to falling back on 
getpid(), as long as the existing codepath is used primarily.

// Martin