search

plasma-umass / DoubleTake

Evidence-based dynamic analysis: a fast checker for memory errors.
MIT License
21 stars 12 forks source link

2 small fixes, ensuring all BSS VMAs are found and prevent data race #20

Closed bpowers closed 9 years ago

bpowers commented 9 years ago

Hi @tongping! I came across two small issues (I think) when reviewing some of how DoubleTake works. more info is in the commit messages below. Let me know if you see any problems, or if I've misunderstood things!

tongping commented 9 years ago

Don't do that:-) I haven't verified it.

Tongping Liu Assistant Professor CS, University of Texas at San Antonio http://www.cs.utsa.edu/~tongpingliu/

On Wed, Aug 5, 2015 at 10:50 AM, Emery Berger notifications@github.com wrote:

Merged #20 https://github.com/plasma-umass/DoubleTake/pull/20.

— Reply to this email directly or view it on GitHub https://github.com/plasma-umass/DoubleTake/pull/20#event-374347928.

emeryberger commented 9 years ago

I looked over the patches and they look solid (and are important fixes).

Professor Emery Berger College of Information and Computer Sciences University of Massachusetts Amherst www.emeryberger.org, @emeryberger

On Wed, Aug 5, 2015 at 8:58 AM, Tongping Liu notifications@github.com wrote:

Don't do that:-) I haven't verified it.

Tongping Liu Assistant Professor CS, University of Texas at San Antonio http://www.cs.utsa.edu/~tongpingliu/

On Wed, Aug 5, 2015 at 10:50 AM, Emery Berger notifications@github.com wrote:

Merged #20 https://github.com/plasma-umass/DoubleTake/pull/20.

— Reply to this email directly or view it on GitHub https://github.com/plasma-umass/DoubleTake/pull/20#event-374347928.

— Reply to this email directly or view it on GitHub https://github.com/plasma-umass/DoubleTake/pull/20#issuecomment-128049515 .

tongping commented 9 years ago

I have confirmed these two changes. Changes on selfmap.h has to be fixed a little bit.

The race won't happen in the second case. But the changes do make the code more cleaner.

Done.

Tongping Liu Assistant Professor CS, University of Texas at San Antonio http://www.cs.utsa.edu/~tongpingliu/

On Wed, Aug 5, 2015 at 12:08 PM, Emery Berger notifications@github.com wrote:

I looked over the patches and they look solid (and are important fixes).

Professor Emery Berger College of Information and Computer Sciences University of Massachusetts Amherst www.emeryberger.org, @emeryberger

On Wed, Aug 5, 2015 at 8:58 AM, Tongping Liu notifications@github.com wrote:

Don't do that:-) I haven't verified it.

Tongping Liu Assistant Professor CS, University of Texas at San Antonio http://www.cs.utsa.edu/~tongpingliu/

On Wed, Aug 5, 2015 at 10:50 AM, Emery Berger notifications@github.com wrote:

Merged #20 https://github.com/plasma-umass/DoubleTake/pull/20.

— Reply to this email directly or view it on GitHub https://github.com/plasma-umass/DoubleTake/pull/20#event-374347928.

— Reply to this email directly or view it on GitHub < https://github.com/plasma-umass/DoubleTake/pull/20#issuecomment-128049515> .

— Reply to this email directly or view it on GitHub https://github.com/plasma-umass/DoubleTake/pull/20#issuecomment-128072713 .

bpowers commented 9 years ago

Thanks for taking a look and confirming!

bpowers commented 9 years ago

just so I can do better in the future @tongping - what was the problem with the isGlobals test?