| Message ID | 20190708140816.1334640-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] drm/amd/powerplay: smu_v11_0: fix uninitialized variable use | expand |
On Mon, Jul 8, 2019 at 5:02 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > On Mon, Jul 08, 2019 at 04:07:58PM +0200, Arnd Bergmann wrote: > > /* if don't has GetDpmClockFreq Message, try get current clock by SmuMetrics_t */ > > - if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) > > + if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) { > > ret = smu_get_current_clk_freq_by_table(smu, clk_id, &freq); > > - else { > > + if (ret) > > + return ret; > > I am kind of surprised that this fixes the warning. If I am interpreting > the warning correctly, it is complaining that the member > get_current_clk_freq_by_table could be NULL and not be called to > initialize freq and that entire statement will become 0. If that is the > case, it seems like this added error handling won't fix that problem, > right? No, I'm fairly sure this is the right fix. Looking at the whole function: | static int smu_v11_0_get_current_clk_freq(struct smu_context *smu, | enum smu_clk_type clk_id, | uint32_t *value) |{ | int ret = 0; | uint32_t freq; | | if (clk_id >= SMU_CLK_COUNT || !value) | return -EINVAL; | | /* if don't has GetDpmClockFreq Message, try get current clock by SmuMetrics_t */ | if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) { | ret = smu_get_current_clk_freq_by_table(smu, clk_id, &freq); Here &freq may or may not get initialized | } else { | ret = smu_send_smc_msg_with_param(smu, SMU_MSG_GetDpmClockFreq, | (smu_clk_get_index(smu, clk_id) << 16)); | if (ret) | return ret; | | ret = smu_read_smc_arg(smu, &freq); | if (ret) | return ret; Same here, but if it's not initialized, we bail out | } | | freq *= 100; | *value = freq; And here it gets assigned to *value | return ret; |} Arnd
diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c index c3f9714e9047..bd89a13b6679 100644 --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c @@ -1099,9 +1099,11 @@ static int smu_v11_0_get_current_clk_freq(struct smu_context *smu, return -EINVAL; /* if don't has GetDpmClockFreq Message, try get current clock by SmuMetrics_t */ - if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) + if (smu_msg_get_index(smu, SMU_MSG_GetDpmClockFreq) == 0) { ret = smu_get_current_clk_freq_by_table(smu, clk_id, &freq); - else { + if (ret) + return ret; + } else { ret = smu_send_smc_msg_with_param(smu, SMU_MSG_GetDpmClockFreq, (smu_clk_get_index(smu, clk_id) << 16)); if (ret)
A mistake in the error handling caused an uninitialized variable to be used: drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: error: variable 'freq' is used uninitialized whenever '?:' condition is false [-Werror,-Wsometimes-uninitialized] ret = smu_get_current_clk_freq_by_table(smu, clk_id, &freq); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note: expanded from macro 'smu_get_current_clk_freq_by_table' ((smu)->ppt_funcs->get_current_clk_freq_by_table ? (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) : 0) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1114:2: note: uninitialized use occurs here freq *= 100; ^~~~ drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1102:10: note: remove the '?:' if its condition is always true ret = smu_get_current_clk_freq_by_table(smu, clk_id, &freq); ^ drivers/gpu/drm/amd/amdgpu/../powerplay/inc/amdgpu_smu.h:880:3: note: expanded from macro 'smu_get_current_clk_freq_by_table' ((smu)->ppt_funcs->get_current_clk_freq_by_table ? (smu)->ppt_funcs->get_current_clk_freq_by_table((smu), (clk_type), (value)) : 0) ^ drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:1095:15: note: initialize the variable 'freq' to silence this warning uint32_t freq; ^ = 0 Bail out of smu_v11_0_get_current_clk_freq() before we get there. Fixes: e36182490dec ("drm/amd/powerplay: fix dpm freq unit error (10KHz -> Mhz)") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)