212412 2020-05-27 09:40:46 -0700 REGRESSION(r260318): [WPE][GTK] Uninitialized memory read in MemoryPressureMonitor 2020-05-27 14:27:17 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build PC Linux RESOLVED FIXED P2 Normal --- 1 mcatanzaro mcatanzaro aperez bugs-noreply mcatanzaro psaavedra oldest_to_newest 1656393 0 mcatanzaro 2020-05-27 09:40:46 -0700 Hm, I don't fully understand this one: ==9020== Thread 46 PressureMonitor: ==9020== Conditional jump or move depends on uninitialised value(s) ==9020== at 0x6AD5D77: systemMemoryUsedAsPercentage (MemoryPressureMonitor.cpp:246) ==9020== by 0x6AD5D77: WebKit::MemoryPressureMonitor::start()::{lambda()#1}::operator()() const [clone .constprop.0] (MemoryPressureMonitor.cpp:347) ==9020== by 0x6AD6358: WTF::Detail::CallableWrapper<WebKit::MemoryPressureMonitor::start()::{lambda()#1}, void>::call() (Function.h:52) ==9020== by 0xA4ACE98: operator() (Function.h:84) ==9020== by 0xA4ACE98: WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) (Threading.cpp:167) ==9020== by 0xA4F83A8: WTF::wtfThreadEntryPoint(void*) (ThreadingPOSIX.cpp:197) ==9020== by 0xB1AB431: start_thread (pthread_create.c:477) ==9020== by 0x58BF9D2: clone (clone.S:95) ==9020== Uninitialised value was created by a stack allocation ==9020== at 0x6AD5BAB: WebKit::MemoryPressureMonitor::start()::{lambda()#1}::operator()() const [clone .constprop.0] (MemoryPressureMonitor.cpp:330) Zero-initializing the token buffer avoids the warning. I'm not sure, but I *think* it's a false positive. token should be initialized by fscanf up through the first NUL character. Past that can be uninitialized, but that shouldn't affect program execution. So I *think* it's not a real bug, though we should certainly initialize the buffer to suppress the warning. 1656394 1 400342 mcatanzaro 2020-05-27 09:42:38 -0700 Created attachment 400342 Patch 1656573 2 400342 aperez 2020-05-27 14:08:35 -0700 Comment on attachment 400342 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400342&action=review > Source/WebKit/ChangeLog:8 > + I think this is a false-positive, but let's suppress the warning by zero-initializing this Seems so… > Source/WebKit/UIProcess/linux/MemoryPressureMonitor.cpp:234 > + char token[MEMINFO_TOKEN_BUFFER_SIZE + 1] = { 0 }; …because the “token” variable is only used if the “fscanf()” call below returns “2”, meaning that it successfully scanned two elements, and when it scans a string, at always adds a terminating '\0' to the buffer. Anyhoo, let's land this, initializing the variable is good. 1656584 3 ews-feeder 2020-05-27 14:27:16 -0700 Committed r262217: <https://trac.webkit.org/changeset/262217> All reviewed patches have been landed. Closing bug and clearing flags on attachment 400342. 400342 2020-05-27 09:42:38 -0700 2020-05-27 14:27:17 -0700 Patch bug-212412-20200527114237.patch text/plain 1722 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjYyMDIyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDZhNzYzOGY4ZDRmMjNiNDcz ZTdiMjI0N2Y4YjgyMDU1N2FkMWI5ZmUuLmNlMGE3ZDE4M2FmNjQwYjVkYTQ0NWQ4YzU4MDczYTQy M2MzNzdjODkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMjAtMDUtMjcgIE1pY2hhZWwg Q2F0YW56YXJvICA8bWNhdGFuemFyb0Bnbm9tZS5vcmc+CisKKyAgICAgICAgUkVHUkVTU0lPTihy MjYwMzE4KTogW1dQRV1bR1RLXSBVbmluaXRpYWxpemVkIG1lbW9yeSByZWFkIGluIE1lbW9yeVBy ZXNzdXJlTW9uaXRvcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9MjEyNDEyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAg ICAgICAgSSB0aGluayB0aGlzIGlzIGEgZmFsc2UtcG9zaXRpdmUsIGJ1dCBsZXQncyBzdXBwcmVz cyB0aGUgd2FybmluZyBieSB6ZXJvLWluaXRpYWxpemluZyB0aGlzCisgICAgICAgIGJ1ZmZlci4K KworICAgICAgICAqIFVJUHJvY2Vzcy9saW51eC9NZW1vcnlQcmVzc3VyZU1vbml0b3IuY3BwOgor ICAgICAgICAoV2ViS2l0OjpzeXN0ZW1NZW1vcnlVc2VkQXNQZXJjZW50YWdlKToKKwogMjAyMC0w NS0yMSAgWW9zaGlha2kgSml0c3VrYXdhICA8eW9zaGlha2kuaml0c3VrYXdhQHNvbnkuY29tPgog CiAgICAgICAgIFtQbGF5U3RhdGlvbl0gQWRkIG1pbmltYWwgV0tWaWV3IEFQSSB0byBlbmFibGUg VGVzdFdlYktpdEFQSQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbGludXgv TWVtb3J5UHJlc3N1cmVNb25pdG9yLmNwcCBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2xpbnV4 L01lbW9yeVByZXNzdXJlTW9uaXRvci5jcHAKaW5kZXggYzNjYmM4OGUzZWFlYzU1Nzg5ZDczOGI3 YzlmZWI5MWU3MjEzMzUyZS4uMmUxMWVkZmJhOTczNjQ5ZDZlNjhjMWFkODNhN2FhN2Q4OThjMzMz YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvbGludXgvTWVtb3J5UHJlc3N1 cmVNb25pdG9yLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9saW51eC9NZW1vcnlQ cmVzc3VyZU1vbml0b3IuY3BwCkBAIC0yMzEsNyArMjMxLDcgQEAgc3RhdGljIGludCBzeXN0ZW1N ZW1vcnlVc2VkQXNQZXJjZW50YWdlKEZJTEUqIG1lbUluZm9GaWxlLCBGSUxFKiB6b25lSW5mb0Zp bGUsIEMKICAgICBtZW1vcnlBdmFpbGFibGUgPSBtZW1vcnlUb3RhbCA9IG1lbW9yeUZyZWUgPSBh Y3RpdmVGaWxlID0gaW5hY3RpdmVGaWxlID0gc2xhYlJlY2xhaW1hYmxlID0gbm90U2V0OwogCiAg ICAgd2hpbGUgKCFmZW9mKG1lbUluZm9GaWxlKSkgewotICAgICAgICBjaGFyIHRva2VuW01FTUlO Rk9fVE9LRU5fQlVGRkVSX1NJWkUgKyAxXTsKKyAgICAgICAgY2hhciB0b2tlbltNRU1JTkZPX1RP S0VOX0JVRkZFUl9TSVpFICsgMV0gPSB7IDAgfTsKICAgICAgICAgc2l6ZV90IGFtb3VudDsKICAg ICAgICAgaWYgKGZzY2FuZihtZW1JbmZvRmlsZSwgIiUiIFNUUklOR0lGWShNRU1JTkZPX1RPS0VO X0JVRkZFUl9TSVpFKSAicyV6dWtCIiwgdG9rZW4sICZhbW91bnQpICE9IDIpCiAgICAgICAgICAg ICBjb250aW51ZTsK