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

ofNoise() -- Memory bad access issue #5820

Closed
Tracked by #7588
nicoleyimessier opened this issue Dec 14, 2017 · 8 comments
Closed
Tracked by #7588

ofNoise() -- Memory bad access issue #5820

nicoleyimessier opened this issue Dec 14, 2017 · 8 comments
Milestone

Comments

@nicoleyimessier
Copy link
Contributor

When running my app with address sanitizer, it quickly catches a bad access that happens within the noise library.

screen shot 2017-12-14 at 11 28 18 am

@bakercp
Copy link
Member

bakercp commented Oct 11, 2019

@nicoleyimessier Did this cause any additional issues or did you find a solution?

@ofTheo
Copy link
Member

ofTheo commented Oct 11, 2019

Huh. This could be an issue.

Looking through the code for slang_library_noise3 I think the variable ii could be negative as is shown in the screen grab above.

Here is the relevant part in the code:
https://github.com/openframeworks/openFrameworks/blob/master/libs/openFrameworks/utils/ofNoise.h#L327

Going back to ii being set from i and i from xs I don't see anything that would stop any of those from being negative.

If ii is negative then there will be an out of bounds with perm

Would love a sanity check on this though
:)

@alexp-nl
Copy link

Recently, I saw some strange and erratic behaviour in a piece of my openFrameworks code using ofNoise. After a long search, I could trace it back to the problem mentioned above. Try debugging into the call ofNoise(-13.7,14.6) and see what happens. Indices in perm become negative. Replacing all the %256 by &0xff solves this problem. Would be nice to fix this in future releases of of.

@ofTheo
Copy link
Member

ofTheo commented Oct 17, 2023

@alexp-nl - it definitely seems like an issue to me.
for some reason the values I get out of perm are fine on macOS even if ii and jj are negative.

changing them to use &0xFF does fix the negative ii and jj it but the noise curve changes.

Do you get erratic values with this code?

void ofApp::draw(){

	ofSetColor(0);
	
	float t = -1234;

	ofNoFill();
	ofBeginShape();
	for(int i = -1024; i < 1024; i++){
		 int ii = i &0xff;
		 int badi = i % 256;
		
		 float v = ofNoise(-16.7 + ((float)i)*0.005, -14.6 );
		 
		 float x = ofMap(i, -1024, 1024, 0, ofGetWindowWidth());
		 float y = ofGetHeight()/2 + v * 100;
		 
		ofVertex(x, y);
	}
	ofEndShape(false);
}

%256
Screenshot 2023-10-17 at 8 51 41 AM

&0xFF
Screenshot 2023-10-17 at 8 53 28 AM

@ofTheo
Copy link
Member

ofTheo commented Oct 17, 2023

Note for positive values both look the same.

The issue is the fix changes ii and jj like so:

i = 0, i %256 = 0, i &0xFF = 0
i = -1, i %256 = -1, i &0xFF = -255
i = -2, i %256 = -2, i &0xFF = -254

hence the different graphs when negative.

@ofTheo ofTheo added this to the 0.12.1 milestone Oct 17, 2023
@alexp-nl
Copy link

The curve should change, because the %256 version is wrong. It depends on data outside of the perm array. In my case, the erratic behaviour was caused by the changing of this data outside (before) the perm array. So two calls with the same parameters to ofNoise resulted in the different return values.
Try replacing the definition of perm
unsigned char perm[512]
by
std::array<unsigned char,512>perm
and (in MSVS) compiling in debug mode. This catches out of range indices in perm. In the %256 case, this happens quite often, the &0xff version works fine.
Great if the problem it is resolved in of 0.12.1!

@ofTheo ofTheo closed this as completed in 46ff0dd Oct 19, 2023
@ofTheo
Copy link
Member

ofTheo commented Oct 19, 2023

Thanks @alexp-nl - this should be fixed in master / nightly now.
Appreciate you bringing this up and can't believe it took this long to fix 😬

@alexp-nl
Copy link

Thanks @alexp-nl - this should be fixed in master / nightly now. Appreciate you bringing this up and can't believe it took this long to fix 😬

Glad I could help, and thanks to everyone who has made openFrameworks what it is today!

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

4 participants