ptchsize behaves wrong if freecom and ptchsize are built with different toolchain

[boeckmann]

This for example is the case under MacOS (and also Linux?), where FreeCOM is built with OpenWatcom but ptchsize is built with clang. The C preprocessor conditionals then test for the ptchsize compiler, not for the freecom compiler.

[jg1uaa]

I met same problem now.

I built FreeCOM on OpenBSD but obtained command.com does not working. Object size is same as built on Linux/OpenWatcom, but contents of EXE_header is different. Here is:

(on Linux mahcine)

uaa@emeraude:~/freecom$ hexdump -C command.com | head -n 2
00000000  4d 5a a8 00 79 00 31 00  0f 00 98 02 98 02 13 0f  |MZ..y.1.........|
00000010  00 10 00 00 da 89 00 00  20 00 00 00 00 00 00 00  |........ .......|
uaa@emeraude:~/freecom$

(on OpenBSD machine)

uaa@framboise:~/z/freecom$ hexdump -C command.com | head -n 2
00000000  4d 5a a8 00 79 00 31 00  0f 00 80 02 80 02 13 0f  |MZ..y.1.........|
00000010  10 28 00 00 da 89 00 00  20 00 00 00 00 00 00 00  |.(...... .......|
uaa@framboise:~/z/freecom$

The modified object, fSS field of command.com 0x2810 -> 0x1000 by hexedit, was worked.

I suspect GCC specific routine of utils/ptchsize.c (it refers tools/ptchsize.c) calculates invalid value. Maybe, utils/mkinfres.c also needs to be checked.

Does anyone who could obtain working command.com on OSX build? It uses clang environment, I think they meets same problem.

[boeckmann]

Does anyone who could obtain working command.com on OSX build? It uses clang environment, I think they meets same problem.

@jg1uaa you may try my patch from https://github.com/boeckmann/freecom/tree/ptchsize_fix

There is a pull request #170, but it is not merged yet.

[jg1uaa]

@boeckmann I saw your PR and I thought about commit 11069f3 , "Adjust stack and heap for ia16-elf-gcc command.com." itself.

I recognized the commit is for native (DOS) ptchsize.exe building with ia16-elf-gcc.
But now we are discussing about cross (OSX/PC-UNIX) ptchsize.exe with clang/gcc.

I have some questions:

1. is heap fix (11069f3) required for cross ptchsize and mkinfres?
2. if heap size is variable, current stack pointer calculation code in mkinfres(https://github.com/FDOS/freecom/blob/master/utils/mkinfres.c#L134) and ptchsize(https://github.com/FDOS/freecom/blob/master/tools/ptchsize.c#L280) is correct?
3. can we avoid this problem using #ifdef ~ #endif, without adding any extra argument for executable?

The result of mkinfres and ptchsize must be same, what toolchain is used.

[jg1uaa]

(Maybe we have to distinguish ia16-elf-gcc for compiling DOS object from clang/gcc for utilities on PC-UNIX...)

[boeckmann]

Maybe we have to distinguish ia16-elf-gcc for compiling DOS object from clang/gcc for utilities on PC-UNIX...

Yes, I think so. This is what I had in mind with the patch. It makes ptchsize get the compiler used to build FreeCOM via command line instead for (wrongly) checking for the compiler used to build ptchsize itself.

I have some questions:

1. is heap fix (11069f3) required for cross ptchsize and mkinfres?

My conclusion (which might be wrong) is no, as the intention seems to be to test for the compiler used to build FreeCOM itself.

2. if heap size is variable, current stack pointer calculation code in mkinfres(https://github.com/FDOS/freecom/blob/master/utils/mkinfres.c#L134) and ptchsize(https://github.com/FDOS/freecom/blob/master/tools/ptchsize.c#L280) is correct?

Not sure. The mkinfres.c probably should also converted to test for the FreeCOM compiler, not the mkinfres compiler, so #ifdefs should be replaced by command line argument for compiler. I did not verify the actual calculations yet. But this should better be double checked...

3. can we avoid this problem using #ifdef ~ #endif, without adding any extra argument for executable?

Should be possible. For example a FREECOM_COMPILER can be defined via the makefiles and passed to the compiler. This could be tested in ptchsize via #if FREECOM_COMPILER == ... instead of the command line argument. But the effect would be the same, and the ptchsize binary would be "locked" to the specific build combination (which is good or bad, depending on how one looks at it, probably more good than bad).

[jg1uaa]

@boeckmann Thanks, I had to read the code and your comment carefully.

I found only tools/ptchsize.c and utils/mkinfres.c have GCC specific code, and mkfiles/gcc.mak with -DGCC rules for DOS binary built with ia16-elf-gcc.

This GCC is only for ia16-elf-gcc, no need for other toolchains.

Commit 468d7b4 adds OSX support. I don't know why committer adds -DGCC with clang, but I think this makes things worse.

How about to start from removing -DGCC like my commit? jg1uaa@c2bff59

[boeckmann]

This GCC is only for ia16-elf-gcc, no need for other toolchains.

Yes.

Commit 468d7b4 adds OSX support. I don't know why committer adds -DGCC with clang, but I think this makes things worse.

Yes, as it unconditionally assumes that FreeCOM is build wich gcc-ia16. However, it by accident does not seem to do any harm when building with Watcom.

How about to start from removing -DGCC like my commit? jg1uaa@c2bff59

Removing it would break the gcc-ia16 build, I guess. So safest option for now would be to replace this with a command line argument to mkinfres, like I did for ptchsize. Then we have time to get a better understanding how all this is connected and hopefully come up with a solution not needing this stuff.

[jg1uaa]

How about to start from removing -DGCC like my commit? jg1uaa@c2bff59

Removing it would break the gcc-ia16 build, I guess. So safest option for now would be to replace this with a command line argument to mkinfres, like I did for ptchsize. Then we have time to get a better understanding how all this is connected and hopefully come up with a solution not needing this stuff.

Removing -DGCC from mkfiles/watcom.mk does not affect gcc-ia16-elf build.

build.sh includes config.mak (copy of config.std) and !include "$(TOP)/mkfiles/$(COMPILER).mak" controls host/target compiler.

• build.sh gcc refers mkfiles/gcc.mak -> gcc for host and ia16-elf-gcc for target
• build.sh watcom refers mkfiles/watcom.mak -> wcl386/clang/gcc for host and wcc for target

So no need to add harmful -DGCC into mkfiles/watcom.mak, I think.

And, the symbol GCC defined at mkfiles/gcc.mak -DGCC is hard to understand. It is to be replaced with TARGET_USE_GCC_IA16.

[boeckmann]

Removing -DGCC from mkfiles/watcom.mk does not affect gcc-ia16-elf build.

build.sh includes config.mak (copy of config.std) and !include "$(TOP)/mkfiles/$(COMPILER).mak" controls host/target compiler.

* `build.sh gcc` refers mkfiles/gcc.mak -> gcc for host and ia16-elf-gcc for target

* `build.sh watcom` refers  mkfiles/watcom.mak -> wcl386/clang/gcc for host and wcc for target

So no need to add harmful -DGCC into mkfiles/watcom.mak, I think.

Sounds reasonable!

And, the symbol GCC defined at mkfiles/gcc.mak -DGCC is hard to understand. It is to be replaced with TARGET_USE_GCC_IA16.

Yes, this name better expresses its intention!