Hi,

On Wed, 22 Oct 2025, Harish Raja Selvan wrote:

> This patch adds support for ARM64EC builds on Windows by updating .def file
> generation logic for shared libraries and ensuring the correct machine flag
> is passed to lib.exe and dumpbin.exe.

Thanks! This version is nicer than the previous ones, and almost looks 
good to me - but there are a couple of smaller details I'd still like to 
fix up.

See the attached patch for my final fixed up version of this; is that one 
ok for you? If it is, I would prefer going with that. I'm commenting 
inline below about the issues I found:

> diff --git a/compat/windows/makedef b/compat/windows/makedef
> index add8222d13..261e7d463f 100755
> --- a/compat/windows/makedef
> +++ b/compat/windows/makedef
> @@ -48,7 +48,13 @@ trap 'rm -f -- $libname' EXIT
>  if [ -n "$AR" ]; then
>      $AR rcs ${libname} $@ >/dev/null
>  else
> -    lib.exe -out:${libname} $@ >/dev/null
> +    machine_flag=""
> +    case "$LDFLAGS" in
> +    *"/machine:arm64ec"*)

Checking for a literal / here is overly restrictive, as the option can be 
passed as either /machine or -machine. We probably don't need to 
explicitly match either of the flags literally, it may be enough to look 
for "machine:arm64ec".

(Note that I think link.exe may accept these flags in uppercase too, and 
then we wouldn't match it. But it's probably ok to not catch that here...)

> +        machine_flag="-machine:arm64ec"
> +        ;;
> +    esac
> +    lib.exe ${machine_flag} -out:${libname} $@ >/dev/null
>  fi
>  if [ $? != 0 ]; then
>      echo "Could not create temporary library." >&2
> @@ -108,10 +114,19 @@ if [ -n "$NM" ]; then
>                cut -d' ' -f3 |
>                sed -e "s/^${prefix}//")
>  else
> -    dump=$(dumpbin.exe -linkermember:1 ${libname} |
> -              sed -e '/public symbols/,$!d' -e '/^ \{1,\}Summary/,$d' -e
> "s/ \{1,\}${prefix}/ /" -e 's/ \{1,\}/ /g' |
> +    member=1
> +    case "$LDFLAGS" in
> +    *"/machine:arm64ec"*)
> +        member=32
> +        ;;
> +    esac
> +    dump=$(dumpbin.exe -linkermember:${member} ${libname} |
> +              sed -e '/public symbols/,$!d' -e '/^ \{1,\}Summary/,$d' -e
> 's/^[[:space:]]*[0-9A-Fa-f]* //' -e "s/^${prefix}//" -e 's/^#//' |
>                tail -n +2 |
> -              cut -d' ' -f3)
> +              cut -d' ' -f3 |
> +              grep -v '\$exit_thunk$' |
> +              grep -v '\$entry_thunk' |
> +              grep -v '\$exit_thunk')

There's two checks for $exit_thunk here - the later one matches a wider 
pattern than the former one. So the former one can be skipped entirely.

The added sed expression 's/^[[:space:]]*[0-9A-Fa-f]* //' makes the later 
"cut" command redundant, so I suggest removing that for clarity. (In the 
updated patch I refactored this to require 1 or more occurrances of the 
hex digits, and match 1 or more spaces after that as well, to make sure it 
eats up all spaces.

For mingw mode, we use a separate invocation of $NM above. I've extended 
that with an expression to remove the leading '#' as well, and remove the 
entry/exit thunks. With that in place, this works fine for arm64ec mingw 
as well.

>  fi
>  
>  rm ${libname}
> diff --git a/configure b/configure
> index 7828381b5d..fbf5ab38a4 100755
> --- a/configure
> +++ b/configure
> @@ -6040,7 +6040,7 @@ case $target_os in
>          SLIB_INSTALL_LINKS=
>          SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib)'
>          SLIB_INSTALL_EXTRA_LIB='lib$(SLIBNAME:$(SLIBSUF)=.dll.a)
> $(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.def)'
> -        SLIB_CREATE_DEF_CMD='EXTERN_PREFIX="$(EXTERN_PREFIX)"
> AR="$(AR_CMD)" NM="$(NM_CMD)" $(SRC_PATH)/compat/windows/makedef
> $(SUBDIR)lib$(NAME).ver $(OBJS) > $$(@:$(SLIBSUF)=.def)'
> +        SLIB_CREATE_DEF_CMD='LDFLAGS="$(LDFLAGS)"
> EXTERN_PREFIX="$(EXTERN_PREFIX)" AR="$(AR_CMD)" NM="$(NM_CMD)"

This is the mingw version (where we pass NM=...) - here we don't need to 
check LDFLAGS, so we don't need to edit this line at all.

With those changes, in the attached patch, this looks good to me. I also 
extended the commit message a bit to mention the other subtle details in 
this change.

// Martin