Commit 83ec09f66c4548d356423894708e6727aaa39c88
1 parent
85ef2cb6
Do memory checks
Slightly improve memory cleanup in Pl_DCT Make it easier to test with valgrind
Showing
5 changed files
with
68 additions
and
30 deletions
README.maintainer
| @@ -21,34 +21,19 @@ Release Reminders | @@ -21,34 +21,19 @@ Release Reminders | ||
| 21 | LDFLAGS="-fsanitize=address" \ | 21 | LDFLAGS="-fsanitize=address" \ |
| 22 | --enable-werror --disable-shared | 22 | --enable-werror --disable-shared |
| 23 | 23 | ||
| 24 | - * Consider running tests with latest gcc and/or valgrind. To do | ||
| 25 | - this, replace, build with debugging and without shared libraries. | ||
| 26 | - In build, create z and move each executable into z. Then create a | ||
| 27 | - script called exec-z that contains: | ||
| 28 | - | ||
| 29 | - #!/bin/sh | ||
| 30 | - exec valgrind --suppressions=/tmp/a.supp -q \ | ||
| 31 | - `dirname $0`/z/`basename $0` ${1+"$@"} | ||
| 32 | - | ||
| 33 | - Symlink exec-z to each executable. /tmp/a.supp can be populated | ||
| 34 | - with suppressions for libraries, for example: | ||
| 35 | - | ||
| 36 | - { | ||
| 37 | - zlib1 | ||
| 38 | - Memcheck:Cond | ||
| 39 | - fun:inflateReset2 | ||
| 40 | - fun:inflateInit2_ | ||
| 41 | - } | ||
| 42 | - { | ||
| 43 | - index | ||
| 44 | - Memcheck:Cond | ||
| 45 | - fun:index | ||
| 46 | - fun:expand_dynamic_string_token | ||
| 47 | - fun:_dl_map_object | ||
| 48 | - fun:map_doit | ||
| 49 | - } | ||
| 50 | - | ||
| 51 | - You can generate these by running valgrind with --gen-suppressions=yes. | 24 | + As of gcc 6.3.0, this exposes some good things but appears to also |
| 25 | + have some false positive leak reports. Valgrind is more reliable | ||
| 26 | + but also may miss some things that this catches. | ||
| 27 | + | ||
| 28 | + * Consider running tests with latest gcc and/or valgrind. To test | ||
| 29 | + with valgrind: | ||
| 30 | + | ||
| 31 | + ./configure --disable-shared | ||
| 32 | + make -j8 -k VALGRIND=1 | ||
| 33 | + make -k check NO_REBUILD=1 | ||
| 34 | + | ||
| 35 | + This moves each binary into a subdirectory and replaces it with a | ||
| 36 | + link to make/exec-z. See make/exec-z. | ||
| 52 | 37 | ||
| 53 | * Check all open issues in the sourceforge trackers and on github. | 38 | * Check all open issues in the sourceforge trackers and on github. |
| 54 | 39 |
libqpdf/Pl_DCT.cc
| @@ -107,6 +107,25 @@ Pl_DCT::finish() | @@ -107,6 +107,25 @@ Pl_DCT::finish() | ||
| 107 | } | 107 | } |
| 108 | } | 108 | } |
| 109 | 109 | ||
| 110 | +class Freer | ||
| 111 | +{ | ||
| 112 | + public: | ||
| 113 | + Freer(unsigned char** p) : | ||
| 114 | + p(p) | ||
| 115 | + { | ||
| 116 | + } | ||
| 117 | + ~Freer() | ||
| 118 | + { | ||
| 119 | + if (*p) | ||
| 120 | + { | ||
| 121 | + free(*p); | ||
| 122 | + } | ||
| 123 | + } | ||
| 124 | + | ||
| 125 | + private: | ||
| 126 | + unsigned char** p; | ||
| 127 | +}; | ||
| 128 | + | ||
| 110 | void | 129 | void |
| 111 | Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) | 130 | Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) |
| 112 | { | 131 | { |
| @@ -124,6 +143,7 @@ Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) | @@ -124,6 +143,7 @@ Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) | ||
| 124 | # pragma GCC diagnostic pop | 143 | # pragma GCC diagnostic pop |
| 125 | #endif | 144 | #endif |
| 126 | unsigned char* outbuffer = 0; | 145 | unsigned char* outbuffer = 0; |
| 146 | + Freer freer(&outbuffer); | ||
| 127 | unsigned long outsize = 0; | 147 | unsigned long outsize = 0; |
| 128 | jpeg_mem_dest(cinfo, &outbuffer, &outsize); | 148 | jpeg_mem_dest(cinfo, &outbuffer, &outsize); |
| 129 | 149 | ||
| @@ -160,8 +180,6 @@ Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) | @@ -160,8 +180,6 @@ Pl_DCT::compress(void* cinfo_p, PointerHolder<Buffer> b) | ||
| 160 | jpeg_finish_compress(cinfo); | 180 | jpeg_finish_compress(cinfo); |
| 161 | this->getNext()->write(outbuffer, outsize); | 181 | this->getNext()->write(outbuffer, outsize); |
| 162 | this->getNext()->finish(); | 182 | this->getNext()->finish(); |
| 163 | - | ||
| 164 | - free(outbuffer); | ||
| 165 | } | 183 | } |
| 166 | 184 | ||
| 167 | void | 185 | void |
make/exec-z
0 → 100755
| 1 | +#!/bin/sh | ||
| 2 | +# This script is used for valgrind testing. See README.maintainer. | ||
| 3 | + | ||
| 4 | +# Create a suppressions file. This can be updated by running valgrind | ||
| 5 | +# with --gen-suppressions=yes. | ||
| 6 | +test -f /tmp/a.supp || cat > /tmp/a.supp <<EOF | ||
| 7 | +{ | ||
| 8 | + zlib1 | ||
| 9 | + Memcheck:Cond | ||
| 10 | + fun:inflateReset2 | ||
| 11 | + fun:inflateInit2_ | ||
| 12 | +} | ||
| 13 | +{ | ||
| 14 | + index | ||
| 15 | + Memcheck:Cond | ||
| 16 | + fun:index | ||
| 17 | + fun:expand_dynamic_string_token | ||
| 18 | + fun:_dl_map_object | ||
| 19 | + fun:map_doit | ||
| 20 | +} | ||
| 21 | +EOF | ||
| 22 | + | ||
| 23 | +exec valgrind --suppressions=/tmp/a.supp -q \ | ||
| 24 | + `dirname $0`/z/`basename $0` ${1+"$@"} |
make/libtool.mk
| @@ -102,6 +102,7 @@ endef | @@ -102,6 +102,7 @@ endef | ||
| 102 | # Usage: $(call makebin,objs,binary,ldflags,libs) | 102 | # Usage: $(call makebin,objs,binary,ldflags,libs) |
| 103 | define makebin | 103 | define makebin |
| 104 | $(LIBTOOL) --mode=link $(CXX) $(CXXFLAGS) $(1) -o $(2) $(4) $(3) | 104 | $(LIBTOOL) --mode=link $(CXX) $(CXXFLAGS) $(1) -o $(2) $(4) $(3) |
| 105 | + if [ "$(VALGRIND)" = 1 ]; then make/valgrind-wrap $(2); fi | ||
| 105 | endef | 106 | endef |
| 106 | 107 | ||
| 107 | # Install target | 108 | # Install target |