aslze / asl

A compact C++ cross-platform library including JSON, XML, HTTP, Sockets, WebSockets, threads, processes, logs, file system, CSV, INI files, vectors and matrices, etc.
Other
69 stars 17 forks source link

Duplicating parameters with CmdArgs results in segfault #11

Closed kabukunz closed 5 years ago

kabukunz commented 5 years ago

Not sure if can be considered an error. Anyway... Insert a cmdline param two times crashes the app:

> test/at3d -cf config/readmesh.ini -cf data/checkmesh/cubeandisolatedvertex.obj
> Error reading test/at3d.ini
> Error reading /Users/max/Developer/Stage/Workspace/AutoTools3D/config/at3d.ini
> Reading data/checkmesh/cubeandisolatedvertex.obj
> =================================================================
> ==15094==ERROR: AddressSanitizer: negative-size-param: (size=-1)
>     #0 0x10f642f00 in wrap_memcpy (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1df00)
>     #1 0x114e5da07 in asl::String::substring(int, int) const String.cpp:444
>     #2 0x10f49a281 in asl::IniFile::write(asl::String const&) IniFile.cpp:121
>     #3 0x10f49b252 in asl::IniFile::~IniFile() IniFile.cpp:201
>     #4 0x10f44a054 in Client::execute(asl::CmdArgs&) Client.hpp:89
>     #5 0x10f449712 in main Test.cpp:22
>     #6 0x7fff5ae0eed8 in start (libdyld.dylib:x86_64+0x16ed8)
> 
> 0x6030000004c0 is located 0 bytes inside of 29-byte region [0x6030000004c0,0x6030000004dd)
> allocated by thread T0 here:
>     #0 0x10f681053 in wrap_malloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x5c053)
>     #1 0x114e5c2ce in asl::String::alloc(int) String.cpp:290
>     #2 0x10f49f26b in asl::Array<asl::String>::insert(int, asl::String const&) String.h:121
>     #3 0x10f498eb7 in asl::IniFile::IniFile(asl::String const&, bool) Array.h:279
>     #4 0x10f449ede in Client::execute(asl::CmdArgs&) Client.hpp:79
>     #5 0x10f449712 in main Test.cpp:22
>     #6 0x7fff5ae0eed8 in start (libdyld.dylib:x86_64+0x16ed8)
> 
> SUMMARY: AddressSanitizer: negative-size-param (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x1df00) in wrap_memcpy
> ==15094==ABORTING
> Abort trap: 6

here's the culprit code:

    bool execute(CmdArgs &args)
    {
     .
     .
     .
        // cmdline overwrite for config
        if (args.has("cf"))
        {
            String configFile = args["cf"];
            IniFile checkconfig(configFile); -> line 79
            if (checkconfig.ok())
            {
                finalConfigFile = configFile;
                printf("Reading %s\n", *checkconfig.fileName());
            }
            else
            {
                printf("Error reading %s\n", *checkconfig.fileName());
            }
        } -> line 89

HTH

aslze commented 5 years ago

FYI, if you have multiple repeated options, you can use the () operator to get all values. Operator [] will only give you one (the last). args("option") returns an array with all values for the given option, unlike args["option"]:

Array<String> cfs = args("cf"); // -> ["config/readmesh.ini", "data/checkmesh/cubeandisolatedvertex.obj"]

for(auto cf : cfs)
{
    read(cf);
}

Parsing multiple options does not seem to be the problem. The crash appears in IniFile which seems to be parsing a badly formatted file. I just identified a possible cause that I'll try to fix. It expects an '=' sign which might be missing in the file. Is the ini file correct?

Anyway, are you passing "cubeandisolatedvertex.obj" to the IniFile reader? Is that an INI-like file?

aslze commented 5 years ago

The crash happened in the IniFile destructor. It checks if we have modified or added any values, to save the file in that case. And there it encountered some missing '='.

I fixed that and another case (if there is a line starting with [ but not closing with ]). Now it will be more forgiving.

It's in the latest master version (pull the latest git version or download the repository zip).

kabukunz commented 5 years ago

An example ini file:

[iteration0]
inputfilename=data/alltriscube.obj
source=0
operation=6
[iteration1]
source=0
operation=7
outputfilename=YES
[iteration2]
source=0
operation=4
# end

you're right, the problem is the obj file passed in place of ini one. It is a text file of the form:

# Blender v2.79 (sub 0) OBJ File: 'triplane_cone_triscube_cube8.blend'
# www.blender.org
v -1.000000 -1.000000 1.000000
v -1.000000 1.000000 1.000000
v -1.000000 -1.000000 -1.000000
v -1.000000 1.000000 -1.000000
v 1.000000 -1.000000 1.000000
v 1.000000 1.000000 1.000000
v 1.000000 -1.000000 -1.000000
v 1.000000 1.000000 -1.000000
s off
f 2 3 1
f 4 7 3
f 8 5 7
f 6 1 5
f 7 1 3
f 4 6 8
f 2 4 3
f 4 8 7
f 8 6 5
f 6 2 1
f 7 5 1
f 4 2 6
l 2 0
l 3 2
l 3 7
l 7 5
l 5 4
l 0 4

if I do:

test/at3d -cf data/alltriscube.obj

the error above shows up. Curiously enough, just discovered, if I do:

test/at3d -cf data/

that is passing only a directory without file, debugger shows everything stops at line 79, never returing. Sanitizers are silent.

I'll modify the code so as to manage multiple options the way you showed. Thank you for your help.

aslze commented 5 years ago

Anyway, shouldn't the obj file be assigned to a different option (not -cf) since it is not a config file?

test/at3d -cf config/readmesh.ini -obj data/checkmesh/cubeandisolatedvertex.obj

-cf for config file and -obj for input obj file. Or even without an option:

test/at3d -cf config/readmesh.ini data/checkmesh/cubeandisolatedvertex.obj

There you will have

args.length(); // -> 1: number of free arguments which are not options
args[0]; // -> "data/checkmesh/cubeandisolatedvertex.obj"
args["cf"]; // -> "config/readmesh.ini"