Re: bzr commit into mysql-5.1-innodb branch (marko.makela:3464) Bug#53593

[Davi Arnaut]

On 5/12/10 4:48 PM, Marko Mäkelä wrote:
> Hi Davi,
>
> thank you for your prompt response.
>
> On Wed, May 12, 2010 at 12:17:02PM -0300, Davi Arnaut wrote:
>> Hi Marko,
>>
>> On 5/12/10 10:04 AM, marko.makela@stripped wrote:
>>> #At file:///home/marko/innobase/dev/mysql/5.1-innodb/ based on
>>> revid:marko.makela@stripped
>>>
>>> 3464 Marko Mäkelä 2010-05-12
>>> Implement Valgrind memcheck instrumentation in the MySQL core. (Bug
>>> #53593)
>>>
>>> Note: I would appreciate feedback regarding my changes to the BUILD
>>> scripts.
>>
>> OK.
>>
>>> modified:
>>> BUILD/SETUP.sh
>>> BUILD/build_mccge.sh
>>> BUILD/compile-amd64-valgrind-max
>>> BUILD/compile-pentium-icc-valgrind-max
>>> BUILD/compile-pentium-valgrind-max
>>> BUILD/compile-pentium-valgrind-max-no-ndb
>>> BUILD/compile-pentium64-valgrind-max
>>> configure.in
>>> include/my_sys.h
>>> storage/innobase/include/univ.i
>>> storage/innodb_plugin/include/univ.i
>>> === modified file 'BUILD/SETUP.sh'
>>> --- a/BUILD/SETUP.sh
>>> revid:marko.makela@stripped
>>> +++ b/BUILD/SETUP.sh
>>> revid:marko.makela@stripped
>>> @@ -119,8 +119,9 @@ fi
>>>
>>> # Set flags for various build configurations.
>>> # Used in -valgrind builds
>>> -valgrind_flags="-USAFEMALLOC -UFORCE_INIT_OF_VARS -DHAVE_purify "
>>> +valgrind_flags="-DHAVE_purify "
>>
>> Hum, removal of FORCE_INIT_OF_VARS should be fine -- in case a
>> valgrind false positive is encountered, the suppression file should be
>> used, and we also use the self-initialization trick to handle false
>> positives from GCC.
>
> Like I wrote later in the bug report, I would not remove
> -UFORCE_INIT_OF_VARS after all. If we removed it, LINT_INIT would have
> to be edited to declare the variable uninitialized in Valgrind.

I see, but my point is that FORCE_INIT_OF_VARS isn't really adequate for 
valgrind. Presumably, it is used to silence where valgrind would wrongly 
warn (false positive) about variables being used without being 
initialized. But this should be silenced using the valgrind suppression 
file, otherwise the valgrind warning would be relevant and should be fixed.

Agree? Bottom line, do you see any valgrind warnings if the 
initialization of variables is not forced?

Ideally, LINT_INIT should only be used for tools that give spurious "use 
before initialization" warnings and that don't have their own way of 
suppression those. In the future, we should probably get rid of 
LINT_INIT and only use UNINIT_VAR.

[..]

>
> One last thing: Which tree should this be pushed to? So far, our
> recently created mysql-5.1-innodb and mysql-trunk-innodb trees have been
> "unidirectional", that is, only changes from InnoDB have been merged to
> the "main" tree. Can I push this to the -innodb trees without fearing
> conflicts at merge time? I hope the trees have not diverged too much
> during the few weeks of *-innodb existence.

Yes, mysql-5.1-innodb is fine. You aren't touching places that change 
very often.

Regards,

Davi Arnaut