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

malloc creates "random" uninitialized pointers in plugin_data. Crashes allpass_1895 and maybe others. #17

Closed
caoliver opened this issue Jun 23, 2015 · 10 comments

Comments

@caoliver
Copy link

If one activates a initialized LADSPA allpass delay plugin with max_delay unconnected, the host crashes with SIGSEGV by chasing plugin_data->max_delay which will contain an unpredictable value, and the NULL test can't guard against that. I don't know how many other plugins suffer the same fault, but perhaps makestub.pl should generate calloc rather than malloc so that pointers are NULL by default.

This is the complaint NerdyProjects raised earlier. BTW: Alsa Modular Synth uses LADSPA plugins, and simply disregarding this issue is not a good thing to do.

@swh
Copy link
Owner

swh commented Jun 23, 2015

I belive 17031ca should address this. Can you test?

@caoliver
Copy link
Author

This seems to be working. I did run into some problems on the build though. faf0a93 seems to beak the build on systems with a more recent libtools. Also autogen.sh expects to find libtoolize in the current directory by the -x test. Maybe that test isn't needed. Last, 422ee38 seems to cause problems with gcc 4.8 and glib 2.17 (at least on Slackware); the deprecated init and fini seem to work. I don't know if this is just some wart with dlopen or something deeper.

@swh
Copy link
Owner

swh commented Jun 24, 2015

OK, I've reverted 422ee38, but I'm a bit worried it will break on some newer libc-s.

Do you know which bit of faf0a93 breaks the build? Someone else reported problems with auto*, but it's too many years since I worked on it to know what's likely to be the issue.

@caoliver
Copy link
Author

On Wed, 24 Jun 2015 02:33:47 -0700
Steve Harris notifications@github.com wrote:

OK, I've reverted 422ee38, but I'm a bit worried it will break on some newer libc-s.

You're correct to be worried. There's something deeper going on. I just tried a simple example, and
the GNU cc attributes flags are doing what they ought to. I'm going to try digging some more and look at the makestub generated code to see if something jumps out at me. It is clear though that as it stands, neither ams nor analyseplugin are able to get the descriptor table.

The libtool complaint is "libtool: link: unsupported hardcode properties" I'm not a libtool/autotools guru. I just git bisected until I found something that let me compile. I don't think both troubles are related, but I'll dig and get back to you.

BTW: thanks for the fix on Bode. I was worried that the scratchy nastiness was just an inevitable DSP artifact, but it sounds great now.

Update:
Eureka! The bug was different than I thought. Revert the reversion. [oopsie] Makefile.am has --nostartfiles sent to the compiler, and that's what's inhibiting the ctor and dtor functions from getting called. It is odd that it's not breaking the "old school" init/fini functions though. See section 5.2 of http://www.tldp.org/HOWTO/pdf/Program-Library-HOWTO.pdf

@swh
Copy link
Owner

swh commented Jun 25, 2015

OK, I've put back what I think is the same code con/de-structor code (modulo there was a typo). Can you test it?

@caoliver
Copy link
Author

On Thu, 25 Jun 2015 06:24:58 -0700
Steve Harris notifications@github.com wrote:

OK, I've put back what I think is the same code co/de-structor code (modulo there was a typo). Can you test it?

Without changing line 46 in Makefile.am from:

AM_LDFLAGS = -module -avoid-version -Wc,-nostartfiles

to:

AM_LDFLAGS = -module -avoid-version

the reversion just takes us back where we started. The point of the post and the LDP reference was to mention that -nostartfiles can't be used when building shared libraries.

@swh
Copy link
Owner

swh commented Jun 25, 2015

... I thought you wanted to go back to where we started?

So Makefile.am just needs that option removing?

@swh
Copy link
Owner

swh commented Jun 25, 2015

Perhaps it would be easier if you sent pull requests?

I don't have a test environment set up, so I'm kind of changing things blind.

@caoliver
Copy link
Author

On Thu, 25 Jun 2015 10:24:39 -0700
Steve Harris notifications@github.com wrote:

Perhaps it would be easier if you sent pull requests?

I'm basing my replies on a clone taken on my private machine. Forgive me, but I generally hate web
for dev tools (awkward), and would rather not do an on-line fork just for this one line change. I put up
with this where I need to contribute significantly. How I wish the major git hosts would supply a decent
ssh accessible command line interface.

See line #46 on https://github.com/swh/ladspa/blob/master/Makefile.am

You need to remove the '-Wc,-nostartfiles' in order for the new style ctor/dtors to get seen at load time, otherwise
they won't get run, and the host code won't think the shared library is a LADSPA plug.

Sincere regards,

Christopher Oliver current.input.port@gmail.com

@swh
Copy link
Owner

swh commented Jul 2, 2015

Thanks, fixed in f3c11af

I share your disdain for web interfaces for development, for what it's worth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants