Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No error for invalid namespace name %std #368

Closed
clintonstimpson opened this issue Mar 26, 2015 · 6 comments
Closed

No error for invalid namespace name %std #368

clintonstimpson opened this issue Mar 26, 2015 · 6 comments
Labels

Comments

@clintonstimpson
Copy link
Contributor

For Java, I have a typemap like this:

%typemap(jstype) std::vector<std::string>, const %std::vector<std::string>&, std::vector<std::string>&  "List<String>"

which results in

swig: DOH/base.c:36: DohDelete: Assertion `0' failed.

If I fix the typo by removing the extra '%'

%typemap(jstype) std::vector<std::string>, const std::vector<std::string>&, std::vector<std::string>&  "List<String>"

then it works.

I am expecting a meaningful error message besides the assertion.

@ojwb
Copy link
Member

ojwb commented Mar 27, 2015

I created issue368.i containing:

%typemap(jstype) std::vector<std::string>, const %std::vector<std::string>&, std::vector<std::string>&  "List<String>"

And processed it like so with current git master:

./preinst-swig -java -c++ -module tmp issue368.i

But I get no assertion failure.

Can you supply a complete short self-contained example which demonstrates the assertion failure along with the command line used to process it?

@clintonstimpson
Copy link
Contributor Author

I did the same thing, an issue368.i file containing the one typemap line.

I call it like this
$ /usr/bin/swig -java -c++ -module tmp issue368.i
swig: DOH/base.c:36: DohDelete: Assertion `0' failed.
Aborted (core dumped)
$ echo $?
134

$ /usr/bin/swig -version
SWIG Version 3.0.5
Compiled with g++ [x86_64-redhat-linux-gnu]
Configured options: +pcre
Please see http://www.swig.org for reporting bugs and further information

$ cat /etc/redhat-release
Fedora release 21 (Twenty One)

I even tried this, and I still get the problem:
git clone https://github.com/swig/swig.git
cd swig
./autogen.sh
./configure
make
./preinst-swig -java -c++ -module tmp ~/issue368.i
And I get this:
swig: DOH/base.c:36: DohDelete: Assertion `0' failed.
Aborted (core dumped)

My ~/issue368.i file contains one line:

%typemap(jstype) std::vector<std::string>, const %std::vector<std::string>&, std::vector<std::string>& "List<String>"

@ojwb
Copy link
Member

ojwb commented Mar 27, 2015

Curiously it now fails for me too - not sure what I managed to actually test before, but I can now reproduce. Sorry about that.

It's trying to delete cparse_unknown_directive from CParse/cscanner.c line 808, and it's apparently not a "DOH" object. That's because Swig_copy_string() just uses malloc() - that ought to be NewString() - fixed in 6b6b360. Thanks for spotting that - it's a regression I introduced in 3.0.4.

However, there's still no error for this case. I'm not sure why, but that isn't a (recent) regression, as it doesn't with SWIG 2.0.12 either. As you say, this really should give an error message.

@clintonstimpson
Copy link
Contributor Author

Thanks for fixing the segfault!
I had been using swig 2.0.x for quite some time with this typo, so you're right, there's no recent regression for catching the typo. Its only with the swig upgrade did I start seeing a segfault.

@ojwb
Copy link
Member

ojwb commented Mar 27, 2015

I've now added a regression test too (dba8d4a).

Checking with -debug-typemap I see that without the abort(), SWIG just adds the typemap to its list, % and all, so it simply won't be used. It's essentially the same as if you'd accidentally written xstd::vector there.

I feel we ought to detect cases such as this where the type isn't syntactically valid, though given a very similar typo in the same location couldn't be flagged up, it doesn't seem a major issue.

@ojwb ojwb changed the title assertion with typo No error for invalid namespace name %std Sep 11, 2016
@ojwb ojwb changed the title No error for invalid namespace name %std No error for invalid namespace name %std Sep 11, 2016
@ojwb ojwb added the parser label Sep 11, 2016
@ojwb
Copy link
Member

ojwb commented Sep 11, 2016

Retitling - the assertion failure is fixed, but there's still an issue here.

@ojwb ojwb closed this as completed in a7ff0da Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants