Fix seg fault at initalization in debug mode #1573
Conversation
| @@ -280,8 +280,14 @@ class TagMap { | |||
| for (int i = 0; tag_table[i].tifftag >= 0; ++i) { | |||
| const EXIF_tag_info *eti = &tag_table[i]; | |||
| m_tagmap[eti->tifftag] = eti; | |||
| if (eti->name) | |||
| m_namemap[eti->name] = eti; | |||
There was a problem hiding this comment.
I'm not sure I like this solution. It feels like something fishy is going on here. What is the value of eti->name when it crashes?
There was a problem hiding this comment.
Is this one of those "static initialization order" fiasco type bugs? What if you wrap both tables in a function so they can be initialized on first use instead of on program startup?
There was a problem hiding this comment.
Finally -- I don't think std::string is needed at all here. Can't you swap for string_view instead?
There was a problem hiding this comment.
Finally^2 -- these table seem small enough that I wonder if std::map is needed at all. I bet a good old linear search would run just as fast or faster.
|
OK, let me take another stab at it. I also suspected initialization order fiasco. |
When using gcc 4.8 on centos7, with debug compile, in a certain app, we get core dumps involving SIGSEGV in __memcmp_sse4_1 deep inside operator[](std::string&&). We suspect it's the old "initialization order fiasco." Wrapping the tag table as a static within a function that returns its reference seems to fix things. I also switched from strings to string_view for the table keys.
|
Updated the patch. Moved the static init of the tables into functions that return their reference. Seems to work. I also switched the std::string map keys to string_view. |
|
LGTM! I still think a linear search would probably be fine too (and maybe faster?) But I don't have a good sense of how this is used, or if these tables could get bigger in the future. |
…on#1573) When using gcc 4.8 on centos7, with debug compile, in a certain app, we get core dumps involving SIGSEGV in __memcmp_sse4_1 deep inside operator[](std::string&&). We suspect it's the old "initialization order fiasco." Wrapping the tag table as a static within a function that returns its reference seems to fix things. I also switched from strings to string_view for the table keys.
(Fix discovered by J Robert Ray)
When using gcc 4.8 on centos7, with debug compile, in a certain app, we get core dumps involving SIGSEGV in __memcmp_sse4_1 deep inside operator.
We don't understand it, and in fact suspect a compiler bug. Can't reproduce on other platforms or compiler releases. Sweep it under the rug with this patch, that seems to work around it, though we admit we don't know why.