Bug in unix/vm/sqUnixExternalPrims.c

Hi:

I belief there is a small bug in ioLoadModule as it is currently in the SVN:

http://squeakvm.org/cgi-bin/viewcvs.cgi/trunk/platforms/unix/vm/sqUnixExternalPrims.c?rev=2375&view=auto

Line 314 reads currently like:

       if ((handle= tryLoading("", path)))


The context:
        fdebugf((stderr, "ioLoadModule plugins = %s\n                path = %s\n",
                 squeakPlugins, path));
        if ((handle= tryLoading("", path)))
          return handle;
        *out++= '/';
        *out= '\0';


While it should read:
       if ((handle= tryLoading(path, "")))

At least according to the arguments names and all other uses, that is.

The signature is:
       static void *tryLoading(char *dirName, char *moduleName)


While that stuff is in the #else branch and might not be used by you guys, it is still there, buggy, and actually used at least in the RoarVM.

Best regards
Stefan


--
Stefan Marr
Software Languages Lab
Vrije Universiteit Brussel
Pleinlaan 2 / B-1050 Brussels / Belgium
http://soft.vub.ac.be/~smarr
Phone: +32 2 629 2974
Fax:   +32 2 629 3525

Dear Stefan,

On Oct 10, 2011, at 04:49 , Stefan Marr wrote:

> I belief there is a small bug in ioLoadModule as it is currently in the SVN:

I believe you are wrong.

Line 314 finds libraries that are located in the CWD or implicitly (via LD_LIBRARY_PATH, ld.so.conf, etc...) but which are named without an explicit prefix (lib) or suffix (.so, .dylib).

Line 221 explains why the order of the arguments is correct and cannot be reversed.

Line 315 explains why the goal of line 314 cannot be achieved by anything after line 315 in ioLoadModule.

> it is still there, buggy

What was your test case, expected outcome, and observed incorrect result?

Regards,
Ian

On Oct 10, 2011, at 05:16 , Ian Piumarta wrote:

> Line 315 explains why the goal of line 314 cannot be achieved by anything after

Line 316.  Sorry for the typo.

Hi Ian:

On 10 Oct 2011, at 14:16, Ian Piumarta wrote:
> On Oct 10, 2011, at 04:49 , Stefan Marr wrote:
>
>> I belief there is a small bug in ioLoadModule as it is currently in the SVN:
>
> I believe you are wrong.
Well, perhaps, the agreement that the current logic is not the simplest was reached before, leading to the situation that the code is now in the else branch of an `#if 1` in the SVN.

> What was your test case, expected outcome, and observed incorrect result?

I had the Tilera Linux barking at me, and was wondering why.
(The actual reasons are things I attribute to Tilera compiler bugs, which made me investigate those warnings in the first place. But I wanted to be sure.)
So, I do not have a test case.

The problem is that currently, the heuristic makes up path names in line 221:

   sprintf(libName, "%s%s%s%s", dirName, *prefix, moduleName, *suffix);

and when we do the tryLoading("", path), then we get all the variations of:

   sprintf(libName, "%s%s%s%s", "", *prefix, path, *suffix);


The value of path in my case was a real path:
    tryLoading (dirName=0x35d6c0 "",
    moduleName=0xbfddd65c "/users/smarr/Projects/RoarVM-master/vm/build/")

And for that particular example, adding the prefixes/suffixes makes that OS bark.

Based on that barking, and my naive inspection of the code, I concluded that it is broken.

Best regards
Stefan


--
Stefan Marr
Software Languages Lab
Vrije Universiteit Brussel
Pleinlaan 2 / B-1050 Brussels / Belgium
http://soft.vub.ac.be/~smarr
Phone: +32 2 629 2974
Fax:   +32 2 629 3525