Comments (4)
You don't describe what options you gave to configure, so I'm guessing either
that
you passed in your own -W flags, or that RHEL has patched gcc to default various
warnings to 'on', that are not on by default in gcc. In any case, in an
out-of-the-box (well, tarfile) install, perftools compiles without any warnings
on
every Linux distribution I've tried (about 7).
I've been able to reproduce many of these warnings by adding -Wall -Wextra to
the
configuration options. Thank you for pointing out the flaw in our
distribution, that
we do not enable any warning flags by default. I will fix that for the next
release.
As for the warnings with -Wall -Wextra, I've briefly looked through them, and
am glad
to report none indicate a real problem (as far as I can tell). The two biggest
category of warnings -- signed/unsigned and unused parameters -- are due to
explicit
coding standards we have. For instance, we consider documenting parameters,
even
unused ones, to be a feature rather than a bug. (As for signedness, signed-type
promotion is one of the biggest warts in the design of C, in my opinion, but
I'll
spare this bug report the rant.)
As for the rest of the warnings, most are for ptmalloc, which is third-party
code we
include in the distribution for allow for comparative tests, but which we did
not
write. We have no plans to modify this third-party code. When fixing up the
perftools Makefile, I'll suppress all warnings for those ptmalloc.
The only warning left after all that is this one:
src/heap-profile-table.cc:297: warning: format '%08llx' expects type 'long long
unsigned int', but argument 4 has type 'uintptr_t'
The code actually uses "%08"PRIxPTR, which is the correct formatting code to
use for
uintptr_t, so I have to conclude this is a buglet in the compiler. I'll look
into
this more, and forward the report to the gcc folks if appropriate.
Original comment by [email protected]
on 22 Jul 2007 at 8:19
- Changed state: Accepted
from gperftools.
You're correct, I used -Wall -Wextra with the standard GNU toolchain.
My compiling the code this way is due to my own coding standards. I won't adopt
any third-party library without first examining its documentation and compiling
it
to see how bad the warnings and errors are. Both of those say a lot about how
much I would struggle to use the code, if adopted, because they say a lot about
how much care was taken to create the code.
On the issue of unsigned parameters, I think you should change your
understanding
of your own coding standard. "Documenting parameters, even unused ones" should
not
be interpreted as "we don't care about compiler warnings". Writing clean code
is
not inconsistent with getting clean compiles. For instance, the following code
in
src/base/googleinit.h:
39 GoogleInitializer(const char* name, void_function f) {
40 f();
41 }
generates this warning:
./src/base/googleinit.h:39: warning: unused parameter 'name'
But this can be easily fixed in a manner that continues to document the unused
parameter:
39 GoogleInitializer(const char* /* name */, void_function f) {
40 f();
41 }
and this alternate form does not generate any warnings.
Why do I harp so much on writing code so clean it generates *no* warnings?
It's because if you get into the habit of allowing warnings, then you will
also get into the habit of ignoring them when they show up. And then all
that good advice the compiler gave you about important problems will be
overlooked, lost in the noise of unimportant problems, and you'll spend
hours and days trying to track down mysterious behaviors when you already
had automation in place that had pointed out the problem.
In my own code, I have my post-build log-file analyzer run immediately after
every nightly build, and the results are emailed to me. With clean code
to start with, it's easy to see if any problems have been created, and to
address them quickly each day. In contrast, if you already have 120 of
some type of warning in each night's build, who will bother to go track
down some new ones that show up, or even notice that the count has changed
in the first place?
So on issues, say, of signedness/unsignedness, stop blaming the language.
It is what it is. If it demands that you state your assumptions clearly
to avoid warnings, say by putting appropriate casts in place, then do so.
(The assumption in a signed/unsigned comparison is that you won't be
comparing against negative values. An explicit cast says to both humans
and the compiler that you thought about it and are willing to guarantee
this contract.) This is a one-time cost for each piece of code you clean
up, and once you have the habit, the effort will be repaid many times by
preventing long debug sessions later on.
In light of that, please don't suppress warnings for third-party libraries
you incorporate. It's important for everybody to see potential problems,
not to hide them. Instead, push the upstream provider to distribute clean
code themselves.
On the issue of the pointer format, we have this on our 32-bit platform:
/usr/include/bits/wordsize.h:
#define __WORDSIZE 32
/usr/include/inttypes.h:
# if __WORDSIZE == 64
# define __PRI64_PREFIX "l"
# define __PRIPTR_PREFIX "l"
# else
# define __PRI64_PREFIX "ll"
# define __PRIPTR_PREFIX
# endif
# define PRIxPTR __PRIPTR_PREFIX "x"
so in this code:
296 printed = snprintf(buf + buflen, bufsize - buflen, " 0x%08" PRIxPTR,
297 reinterpret_cast<uintptr_t>(b.stack[d]));
I don't see where the compiler is getting its idea:
src/heap-profile-table.cc:297: warning: long long unsigned int format, different
type arg (arg 4)
that the format contains "ll".
% gcc --version
gcc (GCC) 3.4.6 20060404 (Red Hat 3.4.6-8)
-----------------
I'm also seeing this outright error when I try to compile the library with
the compiler's -g option instead of -O3, to include all the debug symbols:
if gcc -DHAVE_CONFIG_H -I. -I. -I./src -I./src -D_POSIX_C_SOURCE=200112L
-D_BSD_SOURCE -I/build/thirdparty/include -I/build/my_application/include
-DUSE_P
THREADS -g -Wall -Wextra -pthread -MT ptmalloc_unittest2-t-test2.o -MD -MP -MF
".deps/ptmalloc_unittest2-t-test2.Tpo" -c -o ptmalloc_unittest2-t-test2.o `test
-
f 'src/tests/ptmalloc/t-test2.c' || echo './'`src/tests/ptmalloc/t-test2.c; \
then mv -f ".deps/ptmalloc_unittest2-t-test2.Tpo"
".deps/ptmalloc_unittest2-t-test2.Po"; else rm -f
".deps/ptmalloc_unittest2-t-test2.Tpo"; exit 1; fi
src/tests/ptmalloc/malloc-machine.h: In function `mutex_unlock':
src/tests/ptmalloc/malloc-machine.h:81: warning: matching constraint does not
allow a register
src/tests/ptmalloc/malloc-machine.h:81: warning: matching constraint does not
allow a register
src/tests/ptmalloc/malloc-machine.h:81: error: inconsistent operand constraints
in an `asm'
Original comment by [email protected]
on 22 Jul 2007 at 10:03
from gperftools.
} Why do I harp so much on writing code so clean it generates *no* warnings?
I imagine you'll find many more warnings if you use more warnings flags! For
instance, I have no idea what would happen if you tried to compile perftools
with
-pedantic. The next release of perftools will default to using a set of warning
flags that we think is appropriate for perftools.
I respect that you may have a different idea of what warnings are appropriate
to turn
on for your own software projects. However, we're comfortable with the list of
warnings that we enable.
} I don't see where the compiler is getting its idea:
}
} src/heap-profile-table.cc:297: warning: long long unsigned int format,
different
type arg (arg 4)
}
} that the format contains "ll".
It's not taking the definition in inttypes.h, unfortunately, because a necessary
preprocessor #define isn't being made early enough. This will be fixed in the
next
release.
} src/tests/ptmalloc/malloc-machine.h: In function `mutex_unlock':
Yes, this seems to be a bug with the ptmalloc code. If you do not wish to test
tcmalloc against ptmalloc (which is the malloc used by glibc), you can ignore,
or
comment out, builds of this file.
Original comment by [email protected]
on 22 Jul 2007 at 11:23
from gperftools.
The ptmalloc code continued to give problems (other than the ones discussed
here), so
I ended up taking them out of the default build.
I also set up the Makefile to use the same -W flags that we used internally,
and got
rid of warning messages on all platforms that we test on (see INSTALL file for a
list). There are still warnings with the -W settings that you mention above,
but I
think those are unlikely to get fixed, so I'm going to close out this bug.
Original comment by [email protected]
on 17 Aug 2007 at 9:32
- Changed state: Fixed
from gperftools.
Related Issues (20)
- Thread spawning/joining via Windows 64-bit tcmalloc is very slow on our 16-core server. HOT 5
- gperftools cpu profiler does not support multi process? HOT 1
- SEGV following NULL pointer in SLL_PopRange() related to std::ostringstream memory HOT 3
- [waiting patch] The more elegant and effective hook method on Windows 32/64 HOT 1
- [arm] not able to se caller information (stacktrace is never longer than 1 frame) HOT 9
- segmentation fault on OSX 10.8.5 & gcc-4.7.1 HOT 10
- minimal program linked with tcmalloc hangs on osx 10.8.5 & gcc-4.7.1 HOT 8
- ARMv6 compatibility issue HOT 1
- Exception in CentralFreeList::RemoveRange->FetchFromSpans() HOT 6
- TCMalloc crash in Release with VS2013 HOT 4
- support to use with android ndk HOT 2
- Hang while running make check using 2.1 source distrib on RedHat 6.4 64-bit with libunwind-0.99-beta HOT 5
- [contribution welcome] cpu profiler is not supported on windows HOT 2
- On OS X, during process exit, the static DebugMallocImplementation object may be used after destruction by mz_size, etc. HOT 10
- SEGV in tcmalloc HOT 7
- Missing pthread.h include in static_vars.cc blocks compilation on Mac OS 10.9 HOT 9
- Some *.h/*.cc files are lacking the BSD New License boilerplate HOT 2
- TCMALLOC_MAX_TOTAL_THREAD_CACHE_BYTES doesn't affect tcmalloc behavior (was: in tcmalloc, how tcmalloc_max_total_thread_cache_bytes influence the runtime behavior of tcmalloc ?) HOT 4
- No function names when profiling cpu usage HOT 12
- [Patch] Fix typos in unit test scripts HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gperftools.