open-watcom / open-watcom-v2

Open Watcom V2.0 - Source code repository, Wiki, Latest Binary build, Archived builds including all installers for download.
Other
992 stars 163 forks source link

spurious 'pointer or reference truncated' warning from c++ compiler #1326

Closed sezero closed 3 months ago

sezero commented 3 months ago

Test case:

#include <malloc.h>
#include <stdlib.h>

extern void foo(int, float*, int*);

void Sky_ClipPoly (int nump, float *vecs)
{
    const int max_clip_verts = nump + 2;
    const int on_heap = max_clip_verts > 64;
    int *sides;

    sides = (int *) (on_heap ? malloc(max_clip_verts * sizeof(int)) : alloca(max_clip_verts * sizeof(int)));
    foo(nump, vecs, sides);

    if (on_heap) free(sides);
}

If built using wcc386, no warnings.

If built using wpp386, here it is:

$ wpp386 -zq -wx -bm -bt=nt -5s -sg -otexan -fp5 -fpi87 -ei -j -zp8 -fo=test.obj test.cc
test.cc(12): Warning! W005: col(110) pointer or reference truncated

I don't see an actual issue in the code.

jmalak commented 3 months ago

Please what options you used for wcc386 and what is INCLUDE environment variable value and what host?

sezero commented 3 months ago

Please what options you used for wcc386

As I showed above: -zq -wx -bm -bt=nt -5s -sg -otexan -fp5 -fpi87 -ei -j -zp8

and what is INCLUDE environment variable value?

Simply $WATCOM/h

jmalak commented 3 months ago

Thanks for your info. The issue is caused by alloca inline function which allocate memory on stack explicitly. Below is pre-processed code

extern void based(segname("_STACK")) *doalloca(w_size_t __size);

pragma aux __doalloca = \

        "sub esp,eax"   \
    __parm __nomemory [__eax] \
    __value [__esp] \
    __modify __exact __nomemory [__esp]

    sides = (int *) (on_heap ? malloc(max_clip_verts * sizeof(int)) :  (((((max_clip_verts * sizeof(int))+(sizeof(int)-1))&~(sizeof(int)-1))<stackavail())?(__GRO((((max_clip_verts * sizeof(int))+(sizeof(int)-1))&~(sizeof(int)-1))),__doalloca((((max_clip_verts * sizeof(int))+(sizeof(int)-1))&~(sizeof(int)-1)))): (void *)0) );

C++ compiler for some reason wrongly assume segment is far or is not in FLAT Group even if flat memory model is used. Maybe it is from ternary operator that incorrectly compare both pointers as incompatible. I will review C++ compiler and fix the issue.

sezero commented 3 months ago

Thanks. I'll be waiting for the fix.

jmalak commented 3 months ago

It is confirmed, the issue is with ternary operator.

sezero commented 3 months ago

For what it's worth, the issue doesn't exist in ow1.9 or in the builds from the last code from the old p4 repository.

Smaller test case:

#include <malloc.h>
#include <stdlib.h>

#ifdef __cplusplus
extern "C" {
#endif

extern void foo(int, float*, int*);

void Sky_ClipPoly (int nump, float *vecs)
{
    const int on_heap = nump > 64;
    int *sides =  (int *) (on_heap ? malloc(nump * sizeof(int)) : alloca(nump * sizeof(int)));
    foo(nump, vecs, sides);
    if (on_heap) free(sides);
}

#ifdef __cplusplus
}
#endif

Using ow1.9, there is practically no difference between the disassemblies of the created with wcc386 and wpp386.

However, using the latest ow2.0 builds, the disassembly from the c++ build differs a lot from the c-only build.

jmalak commented 3 months ago

Take into account that C and C++ compiler is significantly changed from OW 1.9. C compiler has significant C99 support improvement. Especially C++ compiler was improved to support some C++11 features. From my analysis of problem C++ compiler incorrectly compare based pointer with regular pointer in ternary operator. the issue is

\<target> = (condition) ? malloc() : alloca(); target is near pointer malloc return near pointer alloca return based pointer (near by size) but if it is used (6 bytes) far pointer ternary operator report difference between near pointer and based pointer. In OW 1.9 alloca return near pointer, but OW 2.0 return based pointer (to _STACK segment) for stricter checking It looks like C++ compiler converts first based pointer to far pointer (6 bytes) even if it is only 4 bytes size. It will require review of code for ternary operator processing because size of both pointers is 4 bytes, but based pointer is 6 bytes far pointer if it is used for memory access, but in ternary operator it is assignment only that size is only 4 bytes. It looks like bug in based pointers processing in C++ compiler.

as workaround you could use construct (for 32-bit code only) \<target> = (condition) ? malloc() : (void __near *)alloca();

jmalak commented 3 months ago

I am not sure if it is compiler bug, because I think it is correct warning for different kind of pointers which are significantly different even if same in size (for ternary operator must be same or compatible). I will look on C compiler code why it is not reported because in this case behaviour should be same. It is bug in header file, it is incorrectly defined __doalloca for small data memory models and next is problem with C compiler which doesn't report this issue. C++ compiler is OK.

jmalak commented 3 months ago

It is fixed now in git repository.

sezero commented 3 months ago

OK, will test as soon as a CI build is available.

BTW, changes are to alloca.sp: Am I to assume malloc.h generated using alloca.sp?

jmalak commented 3 months ago

alloca.sp is include file and is used by malloc.h and stdlib.h which both declare alloca.

jmalak commented 3 months ago

You can check with your options. Next try to add -zu option then it should report the issue again. It is correct behaviour -zu option change stack to be far and therefore pointer to stack cannot be assign to near pointer without truncation.

sezero commented 3 months ago

Tried the CI build: it seems to be good. No asm output difference in C and C++ builds, either. Therefore, I guess this can be closed.

P.S.: Using the -zu option resulted in W005 warning, as expected.